[Patch] Add dirent.d_type support to Cygwin 1.7 ?
Christian Franke
Christian.Franke@t-online.de
Wed Nov 26 23:13:00 GMT 2008
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
More information about the Cygwin-patches
mailing list