Re: [PATCH] remote: avoid -Wunused-but-set-variable in gcc with -DNDEBUG
- From
Jeff King <peff@peff.net>
- Date
- Sep 2, 2021, 08:52 UTC
- Message-ID
- <YTCQ5aJXPMTUpKs8@coredump.intra.peff.net>
- In-Reply-To
- <YTCO56kbtQbODDeK@coredump.intra.peff.net>
[dropped Patrick from cc; that address isn't valid anymore, and he's not active in Git development these days]
On Thu, Sep 02, 2021 at 04:44:23AM -0400, Jeff King wrote:
Show 5 quoted lines
> > this breaks the build with -DNDEBUG because the assert gets compiled > > out and therefore the variable used to check is never used > > Right, this is the real point of the patch. Compiling with NDEBUG will > result in a warning.
Taking all of my suggestions together yields something like:
-- >8 -- From: Carlo Marcelo Arenas Belón <carenas@gmail.com> Subject: [PATCH] remote: avoid -Wunused-but-set-variable in gcc with -DNDEBUG
In make_remote(), we store the return value of hashmap_put() and check it using assert(), but don't otherwise use it. If Git is compiled with NDEBUG, then the assert() becomes a noop, and nobody looks at the variable at all. This causes some compilers to produce warnings.
Let's switch it instead to a BUG(). This accomplishes the same thing, but is always compiled in (and we don't have to worry about the cost; the check is cheap, and this is not a hot code path).
Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> Signed-off-by: Jeff King <peff@peff.net> --- remote.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/remote.c b/remote.c index dfb863d808..40e785da38 100644 --- a/remote.c +++ b/remote.c @@ -135,7 +135,7 @@ static inline void init_remotes_hash(void) static struct remote *make_remote(const char *name, int len) { - struct remote *ret, *replaced; + struct remote *ret; struct remotes_hash_key lookup; struct hashmap_entry lookup_entry, *e; @@ -162,8 +162,8 @@ static struct remote *make_remote(const char *name, int len) remotes[remotes_nr++] = ret; hashmap_entry_init(&ret->ent, lookup_entry.hash); - replaced = hashmap_put_entry(&remotes_hash, ret, ent); - assert(replaced == NULL); /* no previous entry overwritten */ + if (hashmap_put_entry(&remotes_hash, ret, ent)) + BUG("hashmap_put overwrote entry after hashmap_get returned NULL"); return ret; }
-- 2.33.0.446.g793367f49a