From: Junio C Hamano Date: Tue, 06 Jan 2009 23:25:47 GMT Subject: Re: [PATCH/RFC v2 2/4] Use 'lstat_cache()' instead of 'has_symlink_leading_path()' Message-ID: <7vocykf24k.fsf@gitster.siamese.dyndns.org> In-Reply-To: Linus Torvalds writes: >> ... 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. > 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 ;-)