Michael Paquier <mich...@paquier.xyz> writes:
> While looking at the source code for more consistency work with error
> messages, I have bumped into a couple of messages which could be
> simplified, as those include in the name of the file manipulated
> basically the same information as the context added.
> I have finished with the attached. Note that for most of the messages,
> I think that the context can be useful, like for example the stats
> temporary file which could be user-defined, so those are left out.
> There are also other cases, say the reorder buffer spill file, where we
> may not know the path worked on, so the messages are kept consistent as
> per HEAD.
> That's a bit less work to do for translators, particularly with
> pg_stat_statements.
+1. Another thing I noticed is that we now have a fair amount of code
like this example in xlog.c:
errno = 0;
pgstat_report_wait_start(WAIT_EVENT_WAL_COPY_READ);
r = read(srcfd, buffer, nread);
if (r != nread)
{
if (r < 0)
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not read file \"%s\": %m",
path)));
else
ereport(ERROR,
(errmsg("could not read file \"%s\": read %d of
%zu",
path, r, (Size) nread)));
}
pgstat_report_wait_end();
}
The short-read ereport has no errcode() call, meaning it will report
XX000, which seems like it's against project policy for foreseeable
errors. In this example ERRCODE_DATA_CORRUPTED seems better.
BTW, isn't the initial "errno = 0" dead code now?
regards, tom lane
