On Saturday 15 February 2025 19:25:21 Lasse Collin wrote:
> I attached preview patches for a new version that uses FindFirstFileW.
> Perhaps these could be the final version, but I would appreciate if you
> had time to comment these shortly at some point (no rush).
> 
> The main change is just one huge patch instead of multiple smaller
> pieces. :-( Unfortunately I think it cannot be split well because many
> changes depend on each other. Perhaps the addition of d_type and
> _readdir_8dot3 could be split but those are fairly small pieces too.
> I suggest reading dirent.c as a file instead of as a diff.
> 
> Some ponderings:
> 
>   - I changed NAME_MAX to (MAX_PATH - 1) * 3 = 777. The commit message
>     explains why.
> 
>   - get_d_type(): This should be conservative in sense that it should
>     never incorrectly return DT_REG, DT_DIR, or DT_LNK. It is fine to
>     return DT_UNKNOWN when unsure. With this in mind, I wonder if
>     FILE_ATTRIBUTE_PINNED and FILE_ATTRIBUTE_UNPINNED are OK in
>     supported_attrs, or if something else could be improved.

Hello, just a very quick reply regarding the attributes and reparse
points as this is is area which I have been studying in last few weeks.

IMHO, we should not use FILE_ATTRIBUTE_* for determining d_type (except
the FILE_ATTRIBUTE_DIRECTORY which defines directory). Those are
basically attributes which do not define file type in POSIX sense at
all. There are still few attributes not defined yet, which can be added
by MS in future. Also there are some attributes which are already used
by ReFS (e.g. SMR one) but not checked in your code.

Ad reparse points, from application point of view, those are just
replacement of file/dir content during I/O operation. So I think that
existence of reparse point should not determinate d_type (except the
IO_REPARSE_TAG_SYMLINK which defines symlink).

>   - Is _readdir_8dot3() worth it? It's not a lot of code, but every
>     little feature adds up. I suppose it at least is more useful than a
>     function that does best-fit mapping.
> 
>   - The supported errors from FindFirstFileW likely lacks something,
>     and the unknown ones are reported as errno = EIO. In practice it
>     hopefully is good enough though. The errno values from _wfindfirst
>     aren't perfect either.
> 
>   - I think it's valuable to say at the top of dirent.h the version of
>     mingw-w64 where the dirent code was revised. I already put 13.0.0
>     there but it might have been too optimistic.
> 
> <limits.h> requires non-standard _POSIX_ to expose NAME_MAX even though
> some other parts of the headers check for _POSIX_C_SOURCE or
> _XOPEN_SOURCE or _GNU_SOURCE etc. This should be improved but I didn't
> attempt it now. While related to dirent, it's a separate issue.
> 
> A quick web search found even a recent example of NAME_MAX and _POSIX_.
> See especially the end of the comment 4:
> 
>     https://sourceware.org/bugzilla/show_bug.cgi?id=32243
> 
> If it exists in MSVC, I guess changing it in mingw-w64 might cause
> incompatibility somewhere, especially if NAME_MAX happened to be used
> in some DLL API header. *sigh*
> 
> -- 
> Lasse Collin


_______________________________________________
Mingw-w64-public mailing list
Mingw-w64-public@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public

Reply via email to