Re: pgwin32_open returning EINVAL

On Thu, Nov 29, 2007 at 09:43:30AM -0300, Alvaro Herrera wrote:
> Magnus Hagander wrote:
> > On Thu, Nov 29, 2007 at 09:09:47AM -0300, Alvaro Herrera wrote:
> > > Magnus Hagander wrote:
> > > > On Wed, Nov 28, 2007 at 05:20:46PM +0100, Magnus Hagander wrote:
> > > > > On Wed, Nov 28, 2007 at 12:24:26PM -0300, Alvaro Herrera wrote:
> > > > > > Martijn van Oosterhout wrote:
> > > > > > > On Wed, Nov 28, 2007 at 11:57:35AM -0300, Alvaro Herrera wrote:
> > > > > > > > Can we do something like this to report the Win32 error code so that the
> > > > > > > > user has a higher chance of figuring out what's going on?
> > > > >
> > > > > We already do something very similar to what you're donig in
> > > > > backend/port/wni32/socket.c :: TranslateSocketError().
> > > > >
> > > > > So I think it's a good idea to do it, yes.
> > > >
> > > > Thinking about this some more, I think this is a better patch - we already
> > > > have a function that takes care of this, including both error and debug
> > > > logging. Anybody disagree? If not, I'll go ahead and apply it...
> > >
> > > Hmm, but this still mixes some error codes. To absolutely get the
> > > Windows error code you would have to be running with DEBUG5, which I
> > > don't think is acceptable for a production system during any interesting
> > > length of time ... Can we have that DEBUG5 message changed to LOG
> > > instead?
> >
> > 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?

If we can disregard that problem, then I think it's good to increase the
level of logging for that one to either NOTICE or LOG.

> (The only case where a message would be logged inappropriately would be
> in file/fd.c if _dosmaperr returned EINTR, but AFAICS we don't map any
> code to that).

No, we don't - the concept of EINTR doesn't really exist on win32, since
there are no signals..

> > But it is true that we have mixed error codes. But we only do that when we
> > know they're actually there. If we have an unknown code, we don't just
> > return it as EINVAL without logging (as open did before) - and that log
> > goes out as LOG.
>
> Yeah, I understand. But for example there are several different cases
> that are mapped to EACCES. In the cases we're currently following,
> having the exact error code could prove crucial to determining the cause
> of the error.

Yeah, agreed.

//Magnus