From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: Carlo Arenas <carenas@gmail.com>,
"brian m. carlson" <sandals@crustytoothpaste.net>,
Jeff King <peff@peff.net>
Subject: New-ish warning in refs.c with GCC (at least 11.2) under -O3
Date: Mon, 15 Nov 2021 18:41:00 +0100 [thread overview]
Message-ID: <211115.86a6i5s4bn.gmgdl@evledraar.gmail.com> (raw)
(Sent earlier as <211115.86ee7hsa58.gmgdl@evledraar.gmail.com>, but I
typo'd the list address)
This happens on "master", but is old, going back to at least v2.25.0 (I
didn't bother to test older versions):
$ gcc --version
gcc (Debian 11.2.0-10) 11.2.0
[...]
$ make refs.o CC=gcc CFLAGS=-O2
* new build flags
CC refs.o
$ make refs.o CC=gcc CFLAGS=-O3
* new build flags
CC refs.o
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: error: argument 2 null where non-null expected [-Werror=nonnull]
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,
| ^~~~~~
cc1: all warnings being treated as errors
make: *** [Makefile:2500: refs.o] Error 1
I don't have time to dig into it right now, but if anyone's
interested...
It seems like the whole business of passing REF_HAVE_{NEW,OLD} is at
least partially redundant to simply checking the relevant
variables. Maybe.
I.e. our entire test suite passes with the patch below. We "could"
unconditionally check the variable name itself, but I haven't dug enough
to see if that's introducing a subtle bug we're not testing for, or just
a lot of redundant work.
diff --git a/refs.c b/refs.c
index d7cc0a23a3b..335244f756f 100644
--- a/refs.c
+++ b/refs.c
@@ -1061,10 +1061,22 @@ struct ref_update *ref_transaction_add_update(
update->flags = flags;
- if (flags & REF_HAVE_NEW)
+ if (new_oid && flags & REF_HAVE_NEW)
oidcpy(&update->new_oid, new_oid);
- if (flags & REF_HAVE_OLD)
+ else if (!new_oid && flags & REF_HAVE_NEW)
+ BUG("would have passed NULL to memcpy() with REF_HAVE_NEW");
+ else if (new_oid && !(flags & REF_HAVE_NEW))
+ oidcpy(&update->new_oid, new_oid);
+ //BUG("missed a memcpy() new_oid due to no REF_HAVE_NEW");
+
+ if (old_oid && flags & REF_HAVE_OLD)
oidcpy(&update->old_oid, old_oid);
+ else if (!old_oid && flags & REF_HAVE_OLD)
+ BUG("would have passed NULL to memcpy() with REF_HAVE_OLD");
+ else if (old_oid && !(flags & REF_HAVE_OLD))
+ oidcpy(&update->old_oid, old_oid);
+ //BUG("missed a memcpy() old_oid due to no REF_HAVE_OLD");
+
update->msg = normalize_reflog_message(msg);
return update;
}
next reply other threads:[~2021-11-16 1:42 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-15 17:41 Ævar Arnfjörð Bjarmason [this message]
2021-11-15 22:39 ` New-ish warning in refs.c with GCC (at least 11.2) under -O3 Jeff King
2021-11-16 20:29 ` Taylor Blau
2021-11-16 21:22 ` Jeff King
2021-11-18 23:23 ` Junio C Hamano
2021-11-19 21:28 ` [PATCH] refs: work around gcc-11 warning with REF_HAVE_NEW Jeff King
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=211115.86a6i5s4bn.gmgdl@evledraar.gmail.com \
--to=avarab@gmail.com \
--cc=carenas@gmail.com \
--cc=git@vger.kernel.org \
--cc=peff@peff.net \
--cc=sandals@crustytoothpaste.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).