On Wed, 15 Feb 2017 15:23:00 -0500 Robert Haas <robertmh...@gmail.com> wrote:
> + ereport(WARNING, > + (errcode(ERRCODE_INTERNAL_ERROR), > + errmsg("corrupted data found in \"%s\"", > + LOG_METAINFO_DATAFILE))); > > elog seems fine here. There's no need for this to be translatable. > Also, I'd change the level to ERROR. > > + errhint("The supported log formats are > \"stderr\"" > + " and \"csvlog\"."))); > > I think our preferred style is not to break strings across lines like > this. > > + log_filepath[strcspn(log_filepath, "\n")] = '\0'; > > We have no existing dependency on strcspn(). It might be better not > to add one just for this feature; I suggest using strchr() instead, > which is widely used in our existing code. Attached is a v29 patch which fixes the above problems. The Syslogger hunk remains to be fixed. I have no plans to do so at this time. Note that since I have to write an "if" anyway, I'm going ahead and raising an error condition when there's no newline in the current_logfiles file. The strcspn() ignored the missing newline. The new code could do so as well by negating the if condition should that be preferable. On a different topic, I pulled from master and now I see that git finds the following untracked: src/bin/pg_basebackup/pg_receivexlog src/bin/pg_resetxlog/ src/bin/pg_xlogdump/ I'd appreciate knowing if I'm doing something dumb on my end to make this happen. Thanks. Karl <k...@meme.com> Free Software: "You don't pay back, you pay forward." -- Robert A. Heinlein
diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c index 963e70c..ae1fa0b 100644 --- a/src/backend/utils/adt/misc.c +++ b/src/backend/utils/adt/misc.c @@ -906,6 +906,7 @@ pg_current_logfile(PG_FUNCTION_ARGS) char *logfmt; char *log_filepath; char *log_format = lbuffer; + char *nlpos; /* The log format parameter is optional */ if (PG_NARGS() == 0 || PG_ARGISNULL(0)) @@ -918,8 +919,7 @@ pg_current_logfile(PG_FUNCTION_ARGS) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("log format \"%s\" is not supported", logfmt), - errhint("The supported log formats are \"stderr\"" - " and \"csvlog\"."))); + errhint("The supported log formats are \"stderr\" and \"csvlog\"."))); } fd = AllocateFile(LOG_METAINFO_DATAFILE, "r"); @@ -947,19 +947,28 @@ pg_current_logfile(PG_FUNCTION_ARGS) if (log_filepath == NULL) { /* - * Corrupted data found, return NULL to the caller and + * Corrupted data found, no space. Return NULL to the caller and * inform him on the situation. */ - ereport(WARNING, - (errcode(ERRCODE_INTERNAL_ERROR), - errmsg("corrupted data found in \"%s\"", - LOG_METAINFO_DATAFILE))); + elog(ERROR, + "missing space character in \"%s\"", LOG_METAINFO_DATAFILE); break; } *log_filepath = '\0'; log_filepath++; - log_filepath[strcspn(log_filepath, "\n")] = '\0'; + nlpos = strchr(log_filepath, '\n'); + if (nlpos == NULL) + { + /* + * Corrupted data found, no newline. Return NULL to the caller + * and inform him on the situation. + */ + elog(ERROR, + "missing newline character in \"%s\"", LOG_METAINFO_DATAFILE); + break; + } + *nlpos = '\0'; if (logfmt == NULL || strcmp(logfmt, log_format) == 0) {
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers