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

[Jeff King <peff@peff.net>]

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

> > @@ -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