Re: [PATCH/RFC] fsck: complain when .gitignore and .gitattributes are symlinks

From: Jeff King <peff@peff.net>
Date: Jan 17, 2019, 21:24 UTC
Message-ID: <20190117212448.GA13100@sigill.intra.peff.net>
In-Reply-To: <xmqq1s5bniuf.fsf@gitster-ct.c.googlers.com>

On Thu, Jan 17, 2019 at 12:13:12PM -0800, Junio C Hamano wrote:

Show 24 quoted lines

> > @@ -966,7 +968,9 @@ int verify_path(const char *path, unsigned mode)
> >  				if (is_hfs_dotgit(path))
> >  					return 0;
> >  				if (S_ISLNK(mode)) {
> > -					if (is_hfs_dotgitmodules(path))
> > +					if (is_hfs_dotgitmodules(path) ||
> > +					    is_hfs_dotgitignore(path) ||
> > +					    is_hfs_dotgitattributes(path))
> >  						return 0;
> >  				}
> >  			}
> > @@ -974,7 +978,9 @@ int verify_path(const char *path, unsigned mode)
> >  				if (is_ntfs_dotgit(path))
> >  					return 0;
> >  				if (S_ISLNK(mode)) {
> > -					if (is_ntfs_dotgitmodules(path))
> > +					if (is_ntfs_dotgitmodules(path) ||
> > +					    is_ntfs_dotgitignore(path) ||
> > +					    is_ntfs_dotgitattributes(path))
> >  						return 0;
> 
> Curious that we already have these helpers, nobody seems to call
> them in the current codebase, and we haven't seen the "these are
> unused" linter message on the list for a while ;-).

Heh. Yeah, I was surprised by that, too. They were added by e7cb0b4455 (is_ntfs_dotgit: match other .git files, 2018-05-11). The original version of my series had the hunks quoted above, and then we backed off on handling them as part of the emergency fix, but I never re-rolled the preparatory patch to get rid of them.

I think they got overlooked because they're not file-local statics, and it's much harder to say "this is never called by any function in another translation unit". You probably have to do analysis on the complete binaries using "nm" or similar. I think maybe Ramsay does that from time to time, but I don't offhand know the correct incantation.

Anyway, it sounds like you like the overall direction. Does that include these verify_path() bits, as well as the fsck part?

-Peff