Re: pgwin32_open returning EINVAL

From: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
To: Magnus Hagander <magnus(at)hagander(dot)net>
Cc: Martijn van Oosterhout <kleptog(at)svana(dot)org>, Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pgwin32_open returning EINVAL
Date: 2007-11-29 13:10:55
Message-ID: 20071129131055.GH4858@alvh.no-ip.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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)

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jorgen Austvik - Sun Norway 2007-11-29 13:26:24 Re: lo_export and lo_import: paths and servers
Previous Message Gregory Stark 2007-11-29 13:06:36 Re: Table inheritance, unique constraints and foreign key problem