On Wed, 15 Feb 2017 15:23:00 -0500
Robert Haas <[email protected]> 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 <[email protected]>
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 ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
