From: Konstantin Knizhnik <k.knizhnik@postgrespro.ru>
To: Michael Paquier <michael@paquier.xyz>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: readdir is incorrectly implemented at Windows
Date: Fri, 1 Mar 2019 10:23:02 +0300
Message-ID: <000f5395-a787-100b-ceaa-88d86d8be2fc@postgrespro.ru> (raw)
In-Reply-To: <20190301061329.GA31098@paquier.xyz>
References: <2cad7829-8d66-e39c-b937-ac825db5203d@postgrespro.ru>
<20190228021553.GG1617@paquier.xyz>
<20190301061329.GA31098@paquier.xyz>
On 01.03.2019 9:13, Michael Paquier wrote:
> On Thu, Feb 28, 2019 at 11:15:53AM +0900, Michael Paquier wrote:
>> Could you add it to the next commit fest as a bug fix please? I think
>> that I will be able to look at that in details soon, but if not it
>> would be better to not lose track of your fix.
> Okay, I have looked at your patch. And double-checked on Windows. To
> put it short, I agree with the approach you are taking. I have been
> curious about the mention to MinGW in the existing code as well as in
> the patch you are proposing, and I have checked if what your patch and
> what the current state are correct, and I think that HEAD is wrong on
> one thing.
>
> First mingw64 code can be found here:
> https://sourceforge.net/p/mingw-w64/mingw-w64/ci/master/tree/
> https://github.com/Alexpux/mingw-w64/
>
> Then, the implementation of readdir/opendir/closedir can be found in
> mingw-w64-crt/misc/dirent.c. Looking at their implementation of
> readdir, I can see two things:
> 1) When beginning a search in a directory, _tfindfirst gets used,
> which returns ENOENT as error if no matches are found:
> https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/findfirst-functions?view=vs-2017
> So from this point of view your patch is right: you make readdir()
> return errno=0 which matches the normal *nix behaviors. And MinGW
> does not do that.
> 2) On follow-up lookups, MinGW code uses _tfindnext, and actually
> *enforces* errno=0 when seeing ERROR_NO_MORE_FILES. So from this
> point of view the code's comment in HEAD is incorrect as of today.
> The current implementation exists since 399a36a so perhaps MinGW was
> not like that when dirent.c has been added in src/port/, but that's
> not true today. So let's fix the comment at the same time.
>
> Attached is an updated patch with my suggestions. Does it look fine
> to you?
Yes, certainly.
>
> Also, I think that we should credit Yuri Kurenkov for the discovery of
> the issue, with yourself, Konstantin, as the author of the patch.
> Are there other people involved which should be credited? Like
> Grigory?
Yes, Yuri Kurenkov and Grigory Smalking did a lot in investigation of
this problem.
(the irony is that the problem detected by Yuri was caused by another
bug in pg_probackup, but we thought
that it was related with permissions and come to this issue).
> --
> Michael
--
Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: k.knizhnik@postgrespro.ru, michael@paquier.xyz
Subject: Re: readdir is incorrectly implemented at Windows
In-Reply-To: <000f5395-a787-100b-ceaa-88d86d8be2fc@postgrespro.ru>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox