Magnus Hagander wrote:
> On Thu, Nov 29, 2007 at 09:43:30AM -0300, Alvaro Herrera wrote:
> > Magnus Hagander wrote:
> > > Maybe. I'm concerned we might end up logging a whole lot more, for cases
> > > where it's not an actual error. For example, a file that doesn't exist
> > > doesn't necessarily mean it's an error... I don't want to have to go
> > > through all code-paths that end up calling that function to see if that
> > > may
> > > be so...
> >
> > I just checked. I see there are only five callers. In three cases (two
> > in file/fd.c and one in port/dirent.c), there is at a single error code
> > which is "possibly expected". It is taken care of without calling
> > _dosmaperr at all. In syslogger.c there are two possibly expected error
> > codes, dealt with in the same way. And the last caller is
> > port/getrusage.c, which has no possibly-expected error code.
> >
> > So I don't think this is a concern -- whenever _dosmaperr is called, a
> > "true" error message is already going to be logged.
>
> What about all points that call readdir() which maps to that acll in
> port/dirent.c?
Sorry, I don't follow. I think the expected case is that FindNextFile
fails with ERROR_NO_MORE_FILES when there are no more files, on which
case we don't call _dosmaperr.
...
Oh, I see what you mean: for the unexpected cases that readdir() does
call _dosmaperr, readdir returns NULL but what does the caller do?
The good news is that most callers of readdir are in frontend programs:
pg_standby, initdb, pg_resetxlog. There are two callers in the backend:
file/fd.c again, which already calls ereport(ERROR) if anything weird
happen, and pgfnames() which also logs a WARNING.
Callers in frontend programs are not a problem, because the current
_dosmaperr already calls fprintf(stderr) with the code mapping message
in all cases.
Hmm, I just noticed a bug in those fprintf calls -- they are missing
the terminating newline. Please change that too if you're going to
patch this part of the code. In order to avoid translation problems, I
think it should be like this:
fprintf(stderr, _("mapped win32 error code %lu to %d" "\n"), e, errno);
Thanks!
--
Alvaro Herrera http://www.amazon.com/gp/registry/CTMLCN8V17R4
One man's impedance mismatch is another man's layer of abstraction.
(Lincoln Yeoh)
---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend
