Re: [PATCH/RFC v2 2/4] Use 'lstat_cache()' instead of 'has_symlink_leading_path()'

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 6, 2009, 23:25 UTC
Message-ID: <7vocykf24k.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <alpine.LFD.2.00.0901061304280.3057@localhost.localdomain>

Linus Torvalds <torvalds@linux-foundation.org> writes:

Show 23 quoted lines

>> ...  The previously used call:
>> 
>>    has_symlink_leading_path(len, name);
>> 
>> should be identically with the following call to lstat_cache():
>> 
>>    lstat_cache(len, name,
>>                LSTAT_SYMLINK|LSTAT_DIR,
>>                LSTAT_SYMLINK);
>
> I think the new interface looks worse.
>
> Why don't you just do a new inline function that says
>
> 	static inline int has_symlink_leading_path(int len, const char *name)
> 	{
> 		return lstat_cache(len, name,
> 			LSTAT_SYMLINK|LSTAT_DIR,
> 			LSTAT_SYMLINK);
> 	}
>
> and now you don't need this big patch, and people who don't care about 
> those magic flags don't need to have them. End result: more readable code.

Excellent.

Not that I did not think a backward compatible macro is much easier to read; after all, ce/ie you mention is a refactorizaton I did myself.

What I didn't think of was that posing the above question is a much better way to extract a clear explanation why some of these lstat_cache() calls have LSTAT_NOENT and some of them don't from the author. It is much better way than my earlier attempt to do so.

Show 5 quoted lines

> This is how git has done pretty much all "generalized" versions. See the 
> whole ce_modified() vs ie_modified() thing: they're the same function, 
> it's just that 'ce_modified()' is the traditional simpler interface that 
> works on the default index, while ie_modified() is the "full" version that 
> takes all the details that most uses don't even want to know about.

Yup, thanks for a praise ;-)