On Tue, Sep 24, 2019 at 02:08:53AM -0700, Denton Liu wrote:
> When we ran `make hdr-check`, we got the following warning on Arch Linux:
>
> pack-bitmap.h:20:19: error: ‘BITMAP_IDX_SIGNATURE’ defined but not used
> [-Werror=unused-const-variable=]
> 20 | static const char BITMAP_IDX_SIGNATURE[] = {'B', 'I', 'T', 'M'};
> | ^~~~~~~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
>
> "Use" the BITMAP_IDX_SIGNATURE variable by making the size of
> bitmap_disk_header.magic equal to the size of BITMAP_IDX_SIGNATURE. An
> alternative was to simply add MAYBE_UNUSED. However, this design was
> chosen because we eliminate the magic number (4) in the process.
Yeah, I think this is a fine fix. Since it's needed in only a few files,
the more usual fix would be to not define it in the header in the first
place. Something like:
extern const char BITMAP_IDX_SIGNATURE[4];
or whatever. But I think it's nice to keep it all together.
> I'm tacking this patch on since this warning didn't show up until I
> compiled it on gcc 9.1.0.
Curiously, I _don't_ see the warning with gcc 9.2.1. By my reading of
the manpage, this should be triggered by -Wunused-const-variable=2, but
not by "1" (the difference being whether it triggers for stuff in header
files). And only the latter is triggered by -Wall or -Wextra.
But another weirdness is that hdr-check is directly compiling the header
files. So I guess that fools it. But we don't pass any of the extra
diagnostic options there. Have you put "-Wall" into your $(CC)?
Perhaps a more realistic hdr-check would be:
{
echo '#include "git-compat-util.h"'
echo '#include "$<"'
} >$*.hcc
$(QUIET_HDR)$(CC) $(ALL_CFLAGS) -o /dev/null -c -xc $*.hcc
-Peff
