From 49f1eb3b345ca95900e29b70a18302d700c9cd1e Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 19 Nov 2021 16:28:30 -0500 Subject: refs: work around gcc-11 warning with REF_HAVE_NEW MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Using gcc-11 (or 12) to compile refs.o with -O3 results in: In file included from hashmap.h:4, from cache.h:6, from refs.c:5: In function ‘oidcpy’, inlined from ‘ref_transaction_add_update’ at refs.c:1065:3, inlined from ‘ref_transaction_update’ at refs.c:1094:2, inlined from ‘ref_transaction_verify’ at refs.c:1132:9: hash.h:262:9: warning: argument 2 null where non-null expected [-Wnonnull] 262 | memcpy(dst->hash, src->hash, GIT_MAX_RAWSZ); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ In file included from git-compat-util.h:177, from cache.h:4, from refs.c:5: refs.c: In function ‘ref_transaction_verify’: /usr/include/string.h:43:14: note: in a call to function ‘memcpy’ declared ‘nonnull’ 43 | extern void *memcpy (void *__restrict __dest, const void *__restrict __src, | ^~~~~~ That call to memcpy() is in a conditional block that requires REF_HAVE_NEW to be set. But in ref_transaction_update(), we make sure it isn't set coming in: if (flags & ~REF_TRANSACTION_UPDATE_ALLOWED_FLAGS) BUG("illegal flags 0x%x passed to ref_transaction_update()", flags); and then only set it if the variable isn't NULL: flags |= (new_oid ? REF_HAVE_NEW : 0) | (old_oid ? REF_HAVE_OLD : 0); So it should be impossible to reach that memcpy() with a NULL oid. But for whatever reason, gcc doesn't accept that hitting the BUG() means we won't go any further, even though it's marked with the noreturn attribute. And the conditional is correct; ALLOWED_FLAGS doesn't contain HAVE_NEW or HAVE_OLD, and you can even simplify it to check for those flags explicitly and the compiler still complains. We can work around this by just clearing the disallowed flags explicitly. This should be a noop because of the BUG() check, but it makes the compiler happy. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- refs.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/refs.c b/refs.c index 8b9f7c3a80..ccd33c590d 100644 --- a/refs.c +++ b/refs.c @@ -1091,6 +1091,13 @@ int ref_transaction_update(struct ref_transaction *transaction, if (flags & ~REF_TRANSACTION_UPDATE_ALLOWED_FLAGS) BUG("illegal flags 0x%x passed to ref_transaction_update()", flags); + /* + * Clear flags outside the allowed set; this should be a noop because + * of the BUG() check above, but it works around a -Wnonnull warning + * with some versions of "gcc -O3". + */ + flags &= REF_TRANSACTION_UPDATE_ALLOWED_FLAGS; + flags |= (new_oid ? REF_HAVE_NEW : 0) | (old_oid ? REF_HAVE_OLD : 0); ref_transaction_add_update(transaction, refname, flags, -- cgit v1.2.3