Re: [PATCH] MinGW readdir reimplementation to support d_type

[Marius Storm-Olsen <marius@storm-olsen.com>]

Johannes Sixt said the following on 09.04.2009 22:34:
> 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