Horton

Thread

Commits

  1. Map ERROR_INVALID_NAME to ENOENT in mapping table of win32error.c

  1. Extend win32 error codes to errno mapping in win32error.c

    Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> — 2022-09-27T09:53:04Z

    Hi,

I recently faced an issue on windows where one of the tests was
failing with 'unrecognized win32 error code: 123', see [1]. I figured
out that it was due to a wrong file name being sent to open() system
call (this error is of my own making and I fixed it for that thread).
The failure occurs in dosmaperr() in win32error.c due to an unmapped
errno for win32 error code. The error code 123 i.e. ERROR_INVALID_NAME
says "The file name, directory name, or volume label syntax is
incorrect." [2], the closest errno mapping would be ENOENT. I quickly
looked around for the other win32 error codes [2] that don't have
mapping. I filtered out some common error codes such as
ERROR_OUTOFMEMORY, ERROR_HANDLE_DISK_FULL, ERROR_INSUFFICIENT_BUFFER,
ERROR_NOACCESS. There may be many more, but these seemed common IMO.

Having a right errno mapping always helps recognize the errors correctly.

I'm attaching a patch that maps the above win32 error codes to errno
in win32error.c. I also think that we can add a note in win32error.c
by mentioning the link [2] to revisit the mapping whenever
"unrecognized win32 error code:XXX" error occurs.

Thoughts?

Thanks Michael Paquier for off list chat.

[1] https://www.postgresql.org/message-id/CALj2ACWKvjOO-JzYpMBpk-o_o9CeKGEqMcS=yXf-pC6M+jOkuQ@mail.gmail.com
[2] https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-erref/18d8fbe8-a967-4f1c-ae50-99ca8e491d2d

--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

  2. Re: Extend win32 error codes to errno mapping in win32error.c

    Michael Paquier <michael@paquier.xyz> — 2022-09-28T04:40:08Z

    On Tue, Sep 27, 2022 at 03:23:04PM +0530, Bharath Rupireddy wrote:
> The failure occurs in dosmaperr() in win32error.c due to an unmapped
> errno for win32 error code. The error code 123 i.e. ERROR_INVALID_NAME
> says "The file name, directory name, or volume label syntax is
> incorrect." [2], the closest errno mapping would be ENOENT. I quickly
> looked around for the other win32 error codes [2] that don't have
> mapping.

> I filtered out some common error codes such as
> ERROR_OUTOFMEMORY, ERROR_HANDLE_DISK_FULL, ERROR_INSUFFICIENT_BUFFER,
> ERROR_NOACCESS. There may be many more, but these seemed common IMO.
> 
> Having a right errno mapping always helps recognize the errors correctly.

One important thing, in my opinion, when it comes to updating this
table, is that it could be better to report the original error number
if errno can be somewhat confusing for the mapping.  It is also less
interesting to increase the size of the table for errors that cannot
be reached, or related to system calls we don't use.

ERROR_INVALID_NAME => ENOENT
Yeah, this mapping looks fine.

ERROR_HANDLE_DISK_FULL => ENOSPC
This one maps to various Setup*Error(), as well as
GetDiskFreeSpaceEx().  The former is not interesting, but I can buy
the case of the former for extension code (I've played with that in
the past on WIN32, actually).

ERROR_OUTOFMEMORY => ENOMEM
ERROR_NOACCESS => EACCES
ERROR_INSUFFICIENT_BUFFER => EINVAL
Hmm.  I have looked at our WIN32 system calls and the upstream docs,
but these do not seem to be reachable in our code.
--
Michael

  3. Re: Extend win32 error codes to errno mapping in win32error.c

    Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> — 2022-09-28T05:44:53Z

    On Wed, Sep 28, 2022 at 10:10 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> One important thing, in my opinion, when it comes to updating this
> table, is that it could be better to report the original error number
> if errno can be somewhat confusing for the mapping.

Returning errno = e instead of EINVAL in _dosmaperr() may have an
impact on the callers that do a special handling for errno EINVAL. I
don't think it's a good idea.

> ERROR_INVALID_NAME => ENOENT
> Yeah, this mapping looks fine.

Hm.

> ERROR_HANDLE_DISK_FULL => ENOSPC
> This one maps to various Setup*Error(), as well as
> GetDiskFreeSpaceEx().  The former is not interesting, but I can buy
> the case of the former for extension code (I've played with that in
> the past on WIN32, actually).
>
> ERROR_OUTOFMEMORY => ENOMEM
> ERROR_NOACCESS => EACCES
> ERROR_INSUFFICIENT_BUFFER => EINVAL
> Hmm.  I have looked at our WIN32 system calls and the upstream docs,
> but these do not seem to be reachable in our code.

IMO, we can add mapping for just ERROR_INVALID_NAME which is an
obvious error code and easy to hit, leaving others. There are actually
many win32 error codes that may get hit in our code base and actually
mapping everything isn't possible.

Please see the v2 patch. I've also added a CF entry -
https://commitfest.postgresql.org/40/3914/ so that the patch gets
tested across.

--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

  4. Re: Extend win32 error codes to errno mapping in win32error.c

    Michael Paquier <michael@paquier.xyz> — 2022-09-28T05:56:05Z

    On Wed, Sep 28, 2022 at 11:14:53AM +0530, Bharath Rupireddy wrote:
> IMO, we can add mapping for just ERROR_INVALID_NAME which is an
> obvious error code and easy to hit, leaving others. There are actually
> many win32 error codes that may get hit in our code base and actually
> mapping everything isn't possible.

Yes.  I am fine to do just that as you have managed to hit it during
development.  The others may have more opinions to offer.
--
Michael

  5. Re: Extend win32 error codes to errno mapping in win32error.c

    Michael Paquier <michael@paquier.xyz> — 2022-09-29T06:18:48Z

    On Wed, Sep 28, 2022 at 11:14:53AM +0530, Bharath Rupireddy wrote:
> IMO, we can add mapping for just ERROR_INVALID_NAME which is an
> obvious error code and easy to hit, leaving others.

Okidoki.  Applied the minimalistic version, then.
--
Michael