This is the mail archive of the cygwin-patches mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


Christopher Faylor wrote:
On Wed, Nov 26, 2008 at 10:24:14PM +0100, Christian Franke wrote:
...
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.


OK.



...

+#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?

D_type should only be set to the actual type if this info is available at low cost. This is the case for files/dirs, but not for e.g. Cygwin symlinks. Therefore, DT_UNKNOWN is returned instead and the app must call stat() if this info is required.


To speed up typical 'find' and 'ls -R' operations, it is IMO enough to handle the most common filesystem types (for now).


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.


_DIRENT_HAVE_D_TYPE and 'unsigned char d_type' are the same under Linux: http://www.kernel.org/doc/man-pages/online/pages/man3/readdir.3.html


Christian



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]