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

[Christian Franke]

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