Re: readdir is incorrectly implemented at Windows

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