]> git.ipfire.org Git - thirdparty/git.git/commit
revert: initialize const value
authorJeff King <peff@peff.net>
Mon, 4 Aug 2025 13:00:11 +0000 (09:00 -0400)
committerJunio C Hamano <gitster@pobox.com>
Mon, 4 Aug 2025 14:21:12 +0000 (07:21 -0700)
commit1bad05bacc8b9bf45c3b8f576e2ea514472e5737
tree8178e23d9ff67d467d7c46ae8155a8e62f3c5ff5
parenta1cf0cf13ab3a659ac77b7d749d6e5b11dc0142b
revert: initialize const value

When building with clang-22 and DEVELOPER=1 mode, this warning causes us
to fail compilation:

  builtin/revert.c:114:13: error: default initialization of an object of type 'const char' leaves the object uninitialized [-Werror,-Wdefault-const-init-var-unsafe]
    114 |         const char sentinel_value;
        |                    ^

The compiler is right that this code is a bit funny. We declare a const
value without an initializer. It cannot be assigned to because of the
const, but without an initializer it has no predictable value. So as a
variable it can never have any useful function, and if we tried to look
at it, we'd get undefined behavior.

But it does have a function. We never use its value, but rather use its
address as a sentinel value for some other variables:

        const char *gpg_sign = &sentinel_value;

...maybe set gpg_sign via parse_options...

if (gpg_sign != &sentinel_value)
...we got a non-default value...

Normally we'd use NULL as a sentinel value for a pointer, but it doesn't
work here because we also want to detect --no-gpg-sign, which is marked
by setting the pointer to NULL. We need a separate "this was not
touched" value, which is what this sentinel variable gives us.

So the code is correct as-is, but the sentinel variable itself is funny
enough that it's understandable for a compiler warning to flag it. Let's
try to appease the compiler.

There are a few possible options:

  1. Instead of a variable, we could just construct an artificial
     sentinel address like "1", "-1", etc. I think these technically
     fall afoul of the C standard (even if we do not access them, even
     constructing invalid pointers is not always allowed). But it's also
     something we do elsewhere, and even happens in some standard
     interfaces (e.g., mmap()'s MMAP_FAILED value). It does involve some
     annoying casts, though.

  2. We can mark it as static. That gives it a definite value, but
     perhaps makes people wonder if the static-ness is important, when
     it's not.

  3. We can just give it a value to shut the compiler up, even though
     nobody cares about that value.

I went with (3) here as the smallest and most obvious change.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
builtin/revert.c