On 17/01/2019 21:24, Jeff King wrote:
> 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
