Re: pgwin32_open returning EINVAL - Mailing list pgsql-hackers

From Magnus Hagander
Subject Re: pgwin32_open returning EINVAL
Date
Msg-id 20071129125232.GL8718@svr2.hagander.net
Whole thread Raw
In response to Re: pgwin32_open returning EINVAL  (Alvaro Herrera <alvherre@alvh.no-ip.org>)
Responses Re: pgwin32_open returning EINVAL  (Alvaro Herrera <alvherre@alvh.no-ip.org>)
List pgsql-hackers
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



pgsql-hackers by date:

Previous
From: Alvaro Herrera
Date:
Subject: Re: pgwin32_open returning EINVAL
Next
From: Jorgen Austvik - Sun Norway
Date:
Subject: lo_export and lo_import: paths and servers