On 2025-02-15 Pali Rohár wrote:
> 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.
Adding d_type makes sense only if its value can be determined cheaply.
WIN32_FIND_DATAW provides attributes and the reparse point tag, so
using those is practically free. I suppose any other method would have
much more overhead and thus not worth using for d_type.
It's good enough if d_type can be known most of the time. That is, using
DT_REG and DT_DIR for boring regular local files and directories, and
leaving special cases as DT_UNKNOWN (symlinks can be DT_LNK). Is this
doable with the data available in WIN32_FIND_DATAW?
> 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).
get_d_type returns DT_UNKNOWN for all other reparse points than
IO_REPARSE_TAG_SYMLINK. It might be more conservative than needed, but
at least it shouldn't provide wrong information with reparse points.
It also keeps the code simple.
For comparison, Gnulib uses DT_LNK for all reparse points that don't
have FILE_ATTRIBUTE_DIRECTORY. Cygwin does something more complicated
with reparse points, starting here:
https://www.cygwin.com/cgit/newlib-cygwin/tree/winsup/cygwin/fhandler/disk_file.cc?id=d52d983e5b69457c706c34097a328a7678107b1a#n2370
On 2025-02-16 Pali Rohár wrote:
> FYI, in case you are interested in list of windows attributes, you can
> look at my (updated) email which I sent to linux-fsdevel:
> https://lore.kernel.org/linux-fsdevel/20250215233946.cxznczjjiu7vqazf@pali/
Thanks! Based on this, I removed FILE_ATTRIBUTE_EA from supported_attrs.
I also omitted _PINNED and _UNPINNED. Now there are no HSM attributes in
the list.
This change and three other small patches are attached.
By the way, mingw-w64 headers don't define FILE_ATTRIBUTE_VOLUME but
FILE_ATTRIBUTE_STRICTLY_SEQUENTIAL is there.
There seems to be a bug when opening junctions with both old and new
dirent. The old code fails at readdir with EINVAL. The new code fails
at opendir with EACCES. I don't know how to fix it or how big issue it
is in practice. Both old and new code work fine with directory symlinks.
--
Lasse Collin
From c98d01a049946b8a8f6721754598fd1d03248598 Mon Sep 17 00:00:00 2001
From: Lasse Collin <lasse.col...@tukaani.org>
Date: Sun, 16 Feb 2025 09:49:11 +0200
Subject: [PATCH 4/7] ... Shorten API doc comments
The errno preservation in readdir is two assignments in dirent.c
but there's no need to document it in the API.
Don't repeat the doc about _DIRENT_USE_READDIR_8DOT3.
---
mingw-w64-headers/crt/dirent.h | 6 ------
1 file changed, 6 deletions(-)
diff --git a/mingw-w64-headers/crt/dirent.h b/mingw-w64-headers/crt/dirent.h
index 2f7dd36d6..241c530b9 100644
--- a/mingw-w64-headers/crt/dirent.h
+++ b/mingw-w64-headers/crt/dirent.h
@@ -122,9 +122,6 @@ DIR* __cdecl __MINGW_NOTHROW opendir (const char*);
* Reading more would overflow the internal counters.
* EIO Unknown error, possibly an I/O error.
*
- * As an extension, this implementation of readdir guarantees that errno
- * is preserved (never modified) when a non-NULL value is returned.
- *
* If _DIRENT_USE_READDIR_8DOT3 is defined before <dirent.h> is included,
* then readdir is an alias for _readdir_8dot3. See below how it differs.
*/
@@ -144,9 +141,6 @@ struct dirent* __cdecl __MINGW_NOTHROW readdir (DIR*)
__MINGW_ASM_CALL(_readdir_
* If a 8.3 name is returned, the d_8dot3 member in struct dirent is set
* to 1. Otherwise d_8dot3 is set to 0. readdir and _wreaddir never use
* 8.3 names, thus they always set d_8dot3 to 0.
- *
- * If _DIRENT_USE_READDIR_8DOT3 is defined before <dirent.h> is included,
- * then readdir is an alias for _readdir_8dot3.
*/
struct dirent* __cdecl __MINGW_NOTHROW _readdir_8dot3 (DIR*);
--
2.48.1
From 9c105a7cd0341b20f86876cab74d07d89c1ff4a2 Mon Sep 17 00:00:00 2001
From: Lasse Collin <lasse.col...@tukaani.org>
Date: Sun, 16 Feb 2025 09:56:56 +0200
Subject: [PATCH 5/7] ... Don't call FindClose at the end of dir or error
The old code had _findclose here so it had been converted to FindClose.
The application is likely to call closedir (or rewinddir or seekdir)
soon anyway, so leaving the HANDLE open for a moment is fine, and
makes things slightly simpler.
Now the return value of FindClose is no longer silently ignored
when readdir reaches the end of directory or an error occurs.
closedir checks the status from FindClose and so apps can detect
errors from FindClose.
---
mingw-w64-crt/misc/dirent.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/mingw-w64-crt/misc/dirent.c b/mingw-w64-crt/misc/dirent.c
index c3e24eb24..f25f76776 100644
--- a/mingw-w64-crt/misc/dirent.c
+++ b/mingw-w64-crt/misc/dirent.c
@@ -529,18 +529,13 @@ prepare_next_entry (DIR *dirp)
{
if (FindNextFileW (dirp->dd_handle, &dirp->dd_wfd) == 0)
{
- /* The end of the directory was reached or an error occurred. */
- DWORD find_next_err = GetLastError ();
- (void) FindClose (dirp->dd_handle);
- dirp->dd_handle = INVALID_HANDLE_VALUE;
-
- /* Make telldir return the position where the end of directory or
- * the error occurred. Then update dd_stat to indicate the end of
- * the directory or an error status. */
+ /* The end of the directory was reached or an error occurred.
+ * Make telldir return this position. Then update dd_stat to
+ * indicate the end of the directory or an error status. */
dirp->dd_pos = dirp->dd_stat;
dirp->dd_stat = -1;
- switch (find_next_err)
+ switch (GetLastError ())
{
case ERROR_NO_MORE_FILES:
return dirp->dd_endval;
--
2.48.1
From 45d83542a70a5d3cf8b369b893ad8245e90dbb27 Mon Sep 17 00:00:00 2001
From: Lasse Collin <lasse.col...@tukaani.org>
Date: Sun, 16 Feb 2025 10:06:09 +0200
Subject: [PATCH 6/7] ... prepare_next_entry: Return ENOENT only once if dd_pos
== LONG_MAX
This is how all other errors are already handled. Without this,
readdir would keep failing with ENOENT (which should be OK but it
was inconsistent with handling of other errors).
---
mingw-w64-crt/misc/dirent.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/mingw-w64-crt/misc/dirent.c b/mingw-w64-crt/misc/dirent.c
index f25f76776..4c549b564 100644
--- a/mingw-w64-crt/misc/dirent.c
+++ b/mingw-w64-crt/misc/dirent.c
@@ -515,6 +515,7 @@ prepare_next_entry (DIR *dirp)
/* Too many entries. Treat it as invalid directory stream position.
* It's highly unlikely that there are this many files in a directory,
* so adding code to handle larger directories seems pointless. */
+ dirp->dd_stat = -1;
return ENOENT;
}
--
2.48.1
From c835a4d26867e5fc73da6f6ea463d9a57f22d0db Mon Sep 17 00:00:00 2001
From: Lasse Collin <lasse.col...@tukaani.org>
Date: Sun, 16 Feb 2025 10:49:21 +0200
Subject: [PATCH 7/7] ... Omit _EA, _PINNED, and _UNPINNED from supported_attrs
---
mingw-w64-crt/misc/dirent.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/mingw-w64-crt/misc/dirent.c b/mingw-w64-crt/misc/dirent.c
index 4c549b564..6a60559c3 100644
--- a/mingw-w64-crt/misc/dirent.c
+++ b/mingw-w64-crt/misc/dirent.c
@@ -391,8 +391,12 @@ get_d_type (DWORD attrs, DWORD reparse_tag)
* FILE_ATTRIBUTE_DEVICE
* FILE_ATTRIBUTE_OFFLINE
* FILE_ATTRIBUTE_VIRTUAL
+ * FILE_ATTRIBUTE_EA (overlaps FILE_ATTRIBUTE_RECALL_ON_OPEN)
+ * FILE_ATTRIBUTE_PINNED
+ * FILE_ATTRIBUTE_UNPINNED
* FILE_ATTRIBUTE_RECALL_ON_OPEN
* FILE_ATTRIBUTE_RECALL_ON_DATA_ACCESS
+ * FILE_ATTRIBUTE_STRICTLY_SEQUENTIAL
*/
unsigned int supported_attrs = FILE_ATTRIBUTE_READONLY |
FILE_ATTRIBUTE_HIDDEN |
@@ -407,10 +411,7 @@ get_d_type (DWORD attrs, DWORD reparse_tag)
FILE_ATTRIBUTE_NOT_CONTENT_INDEXED |
FILE_ATTRIBUTE_ENCRYPTED |
FILE_ATTRIBUTE_INTEGRITY_STREAM |
- FILE_ATTRIBUTE_NO_SCRUB_DATA |
- FILE_ATTRIBUTE_EA |
- FILE_ATTRIBUTE_PINNED |
- FILE_ATTRIBUTE_UNPINNED;
+ FILE_ATTRIBUTE_NO_SCRUB_DATA;
if (attrs & ~supported_attrs)
{
--
2.48.1
_______________________________________________
Mingw-w64-public mailing list
Mingw-w64-public@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
