Re: [PATCH] git-compat-util: make git_find_last_dir_sep return a const pointer

[Toon Claes <toon@iotcl.com>]

Jeff King <peff@peff.net> writes:

>> > Looking at strchr()'s declaration in string.h, which is defined like:
>> >
>> >   #  define strchr(S, C)                                          \
>> >     __glibc_const_generic (S, const char *, strchr (S, C))
>> >
>> > I think the answer is probably "yes". But it also doesn't quite solve
>> > our problem. That would give us type-checking of callers of our
>> > function, but we still have to convince the compiler not to complain
>> > about its implementation. For that we'd need to either cast away const
>> > manually, I guess.
>> 
>> That macro depends on Generic selections from C11 [1]. I wasn't sure if
>> Git would like that, given it is conservative with other C features.
>
> We definitely can't rely on it everywhere. But if there is a solution
> that is conditionally compiled, and can kick in only when these extra
> warnings also kick in, that would be OK. Assuming the result is not too
> painful to look at, of course.

So the Git project would be okay to conditionally compile with Generic
selections if the compiler supports it? Seems to me this is the easiest
way forward to silence the errors for users who see these warnings (that
includes me).

> Probably the best path forward for most spots is just fixing the code to
> make it more obvious about its use of const.

Yeah, that's in any case a good idea. I think using Generic selections
should make it easier to locate those.

This is what I've done in an experiment in git-compat-util.h:

    #ifndef find_last_dir_sep
    static inline char *git_find_last_dir_sep_nonconst(char *path)
    {
    	return strrchr(path, '/');
    }
    static inline const char *git_find_last_dir_sep_const(const char *path)
    {
    	return strrchr(path, '/');
    }
    #if __STDC_VERSION__ >= 201112L || (defined(__GNUC__) && __GNUC__ >= 5)
    #define find_last_dir_sep(path) \
    	_Generic((path), \
    		const char *: git_find_last_dir_sep_const((const char *)(path)), \
    		default:      git_find_last_dir_sep_nonconst((char *)(path)))
    #else
    #define find_last_dir_sep(path) git_find_last_dir_sep_nonconst((char *)(path))
    #endif
    #endif

This leads to 28 places where the warning happens:

    bloom.c:515:52: warning: initialization discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    builtin/config.c:855:22: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    builtin/receive-pack.c:1045:19: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    builtin/receive-pack.c:1075:27: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    builtin/receive-pack.c:1098:19: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    builtin/rev-parse.c:278:22: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    builtin/rev-parse.c:339:21: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    builtin/rev-parse.c:343:28: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    builtin/rev-parse.c:347:28: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    convert.c:1189:24: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    convert.c:1212:32: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    convert.c:1223:29: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    dir.c:3526:15: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    http-push.c:1775:44: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    http.c:755:43: warning: initialization discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    pager.c:121:28: warning: initialization discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    pseudo-merge.c:647:16: warning: return discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    range-diff.c:109:50: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    refs/files-backend.c:2199:25: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    revision.c:2135:24: warning: initialization discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    revision.c:2179:14: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    revision.c:2188:14: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    revision.c:2194:14: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    run-command.c:608:32: warning: initialization discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    send-pack.c:184:19: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    send-pack.c:215:27: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    send-pack.c:240:19: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
    transport-helper.c:803:19: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]

I did not look into any of them, but I think you (Collin) have sent out
patches for various of these? But they _should_ managable to address?

-- 
Cheers,
Toon