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

From: Ramsay Jones <ramsay@ramsayjones.plus.com>
Date: Jan 18, 2019, 01:41 UTC
Message-ID: <a0aef5a7-eb69-8dd8-abb7-4db6d1de4a26@ramsayjones.plus.com>
In-Reply-To: <20190117212448.GA13100@sigill.intra.peff.net>

On 17/01/2019 21:24, Jeff King wrote:

Show 38 quoted lines

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

I don't do this "from time to time", but *every* build on all platforms! :-D

As I have mentioned before, I run the script on 'master', 'next' and 'pu', but I don't look at the results for 'master', I simply look at the diffs master->next and next->pu.

I put the output of 'static-check.pl' in the sc, nsc and psc files (guess which files are for which branches!). For example, tonight I find:

    $ wc -l sc nsc psc
      90 sc
      90 nsc
     100 psc
     280 total
    $ diff sc nsc
    $ diff nsc psc
    29a30,32
    > config.o	- repo_config_set
    > config.o	- repo_config_set_gently
    > config.o	- repo_config_set_worktree_gently
    32a36
    > fuzz-commit-graph.o	- LLVMFuzzerTestOneInput
    37a42,43
    > hex.o	- hash_to_hex
    > hex.o	- hash_to_hex_algop_r
    74a81,83
    > sha1-file.o	- hash_algo_by_id
    > sha1-file.o	- hash_algo_by_name
    > sha1-file.o	- repo_has_sha1_file_with_flags
    80a90
    > strbuf.o	- strbuf_vinsertf
    $

BTW, if my memory serves (and it may not), the symbols you refer to came directly into 'master' (via 'maint') as a result of security updates - so I would never have seen them in 'pu' or 'next'. They are, indeed, currently noted in the 'master' branch:

    $ grep is_ntfs_ sc
    path.o	- is_ntfs_dotgitattributes
    path.o	- is_ntfs_dotgitignore
    $ grep is_hfs_ sc
    utf8.o	- is_hfs_dotgitattributes
    utf8.o	- is_hfs_dotgitignore
    $ 
    
ATB,
Ramsay Jones