Re: [PATCH] MinGW readdir reimplementation to support d_type
- From
Marius Storm-Olsen <marius@storm-olsen.com>
- Date
- Apr 10, 2009, 07:50 UTC
- Message-ID
- <49DEFA30.1000101@gmail.com>
- In-Reply-To
- <49DE5BDE.9050709@kdbg.org>
Johannes Sixt said the following on 09.04.2009 22:34:
Show 21 quoted lines
> Marius Storm-Olsen schrieb:
>> +struct mingw_dirent
>> +{
>> + long d_ino; /* Always zero. */
>> + union {
>> + unsigned short d_reclen; /* Always zero. */
>> + unsigned char d_type; /* Reimplementation adds this */
>> + };
>
> VERY sneaky! I was wondering why you could get away without replacing
> opendir and closedir, and why you still defined a replacement
> mingw_DIR that contains the replacement mingw_dirent, until I noticed
> this unnamed union.
>
> Since we don't use d_reclen anywhere in the code, wouldn't you get
> away with
>
> #define d_type d_reclen
>
> unless the type (short vs. char) makes a difference. Or would you say
> that doing that would be even more sneaky?I'm sure it could be done just with a define. However, given the remaining unused variables, I was wondering about also packing in permission bits and file modification time in there, to optimize the status checking even further. That way, on Windows, we would only need one 'readdir' pass to check the whole repository, with no lstats whatsoever. So, this was patch was a 'primer' for that, hence the union with a proper uchar for the d_type.
However, that would also mean a significant change in the status checking code, as it first lstat's ever file in the index, then uses read_directory + lstat's for others. I guess that'll be too big of a change in core code, so the vision is moot?
I'd be ok to just use the define, provided that it compiles cleanly of course, if the above seems too ambitious. :-) I kinda feel like the current code is more clean though :)
-- .marius