Skip to content

Commit 1bad05b

Browse files
peffgitster
authored andcommitted
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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent a1cf0cf commit 1bad05b

File tree

1 file changed

+1
-1
lines changed

1 file changed

+1
-1
lines changed

builtin/revert.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ static int run_sequencer(int argc, const char **argv, const char *prefix,
111111
const char * const * usage_str = revert_or_cherry_pick_usage(opts);
112112
const char *me = action_name(opts);
113113
const char *cleanup_arg = NULL;
114-
const char sentinel_value;
114+
const char sentinel_value = 0; /* value not important */
115115
const char *strategy = &sentinel_value;
116116
const char *gpg_sign = &sentinel_value;
117117
enum empty_action empty_opt = EMPTY_COMMIT_UNSPECIFIED;

0 commit comments

Comments
 (0)