[Patch] Add dirent.d_type support to Cygwin 1.7 ?

Christopher Faylor cgf-use-the-mailinglist-please@cygwin.com
Wed Nov 26 22:11:00 GMT 2008


On Wed, Nov 26, 2008 at 10:24:14PM +0100, Christian Franke wrote:
> This is an experimental patch to add dirent.d_type support to readdir(). It 
> sets d_type to DT_DIR/REG for normal disk directories/files and DT_UNKNOWN 
> in all other cases.
>
> Test result with original find (4.4.0-3) vs. same find rebuild with new 
> sys/dirent.h:
>
> $ export TIMEFORMAT='%1R'
>
> $ time find /cygdrive/c/cygwin >/dev/null
> 30.5
>
> $ time find-with-d_type /cygdrive/c/cygwin >/dev/null
> 9.5
>
> $ time cmd /c dir /a/s 'c:\cygwin' >/dev/null
> 5.2
>
> Due to the missing initialization of '__d_unused1', new programs with 
> d_type support would not be backward compatible.
>
>
> Christian
>

>diff --git a/winsup/cygwin/dir.cc b/winsup/cygwin/dir.cc
>index 30662e6..c200469 100644
>--- a/winsup/cygwin/dir.cc
>+++ b/winsup/cygwin/dir.cc
>@@ -93,6 +93,11 @@ readdir_worker (DIR *dir, dirent *de)
>     }
> 
>   de->d_ino = 0;
>+#ifdef _DIRENT_HAVE_D_TYPE
>+  de->d_type = DT_UNKNOWN;
>+#endif
>+  memset (&de->__d_unused1, 0, sizeof (de->__d_unused1));
>+

I don't see a need for a conditional here.  If this is added Cygwin
supports d_type.

>   int res = ((fhandler_base *) dir->__fh)->readdir (dir, de);
> 
>   if (res == ENMFILE)
>diff --git a/winsup/cygwin/fhandler_disk_file.cc b/winsup/cygwin/fhandler_disk_file.cc
>index e0880f0..c748e24 100644
>--- a/winsup/cygwin/fhandler_disk_file.cc
>+++ b/winsup/cygwin/fhandler_disk_file.cc
>@@ -1677,6 +1677,28 @@ fhandler_disk_file::readdir_helper (DIR *dir, dirent *de, DWORD w32_err,
>       dir->__flags &= ~dirent_set_d_ino;
>     }
> 
>+#ifdef _DIRENT_HAVE_D_TYPE
>+  /* Set d_type if type can be determined from file attributes.
>+     FILE_ATTRIBUTE_SYSTEM ommitted to leave DT_UNKNOWN for old symlinks.
>+     For new symlinks, d_type will be reset to DT_UNKNOWN below.  */
>+  if (attr &&
>+      !(attr & ~( FILE_ATTRIBUTE_NORMAL
>+                | FILE_ATTRIBUTE_READONLY
>+                | FILE_ATTRIBUTE_ARCHIVE
>+                | FILE_ATTRIBUTE_HIDDEN
>+                | FILE_ATTRIBUTE_COMPRESSED
>+                | FILE_ATTRIBUTE_ENCRYPTED
>+                | FILE_ATTRIBUTE_SPARSE_FILE
>+                | FILE_ATTRIBUTE_NOT_CONTENT_INDEXED
>+                | FILE_ATTRIBUTE_DIRECTORY)))
>+    {
>+      if (attr & FILE_ATTRIBUTE_DIRECTORY)
>+        de->d_type = DT_DIR;
>+      else
>+        de->d_type = DT_REG;
>+    }
>+#endif
>+

This is just checking all of the Windows types but none of the Cygwin
types.  Shouldn't it be checking for devices, fifos, and symlinks?
>diff --git a/winsup/cygwin/include/sys/dirent.h b/winsup/cygwin/include/sys/dirent.h
>index 41bfcc1..d782e58 100644
>--- a/winsup/cygwin/include/sys/dirent.h
>+++ b/winsup/cygwin/include/sys/dirent.h
>@@ -18,11 +18,17 @@
> 
> #pragma pack(push,4)
> #if defined(__INSIDE_CYGWIN__) || defined (__CYGWIN_USE_BIG_TYPES__)
>+#define _DIRENT_HAVE_D_TYPE
> struct dirent
> {
>   long __d_version;			/* Used internally */
>   __ino64_t d_ino;
>+#ifdef _DIRENT_HAVE_D_TYPE
>+  unsigned char d_type;
>+  unsigned char __d_unused1[3];
>+#else
>   __uint32_t __d_unused1;
>+#endif

There is even less reason to define and use _DIRENT_HAVE_D_TYPE here.

Why not just define d_type as a __uint32_t?  We don't need to keep the
__d_unused1 around.

cgf



More information about the Cygwin-patches mailing list