From 97b989ee3a9add534d561aefca0f3b65ec26a924 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Wed, 25 Sep 2019 01:20:53 -0700 Subject: [PATCH 1/5] apply.h: include missing header When running `make hdr-check`, we got the following error messages: apply.h:146:22: error: use of undeclared identifier 'GIT_MAX_HEXSZ' char old_oid_prefix[GIT_MAX_HEXSZ + 1]; ^ apply.h:147:22: error: use of undeclared identifier 'GIT_MAX_HEXSZ' char new_oid_prefix[GIT_MAX_HEXSZ + 1]; ^ apply.h:151:33: error: array has incomplete element type 'struct object_id' struct object_id threeway_stage[3]; ^ ./strbuf.h:79:8: note: forward declaration of 'struct object_id' struct object_id; ^ 3 errors generated. make: *** [apply.hco] Error 1 Include the missing "hash.h" header to fix these errors. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- apply.h | 1 + 1 file changed, 1 insertion(+) diff --git a/apply.h b/apply.h index a795193435..da3d95fa50 100644 --- a/apply.h +++ b/apply.h @@ -1,6 +1,7 @@ #ifndef APPLY_H #define APPLY_H +#include "hash.h" #include "lockfile.h" #include "string-list.h" From b06fdead04b838a2522776ce7e3465146d84a224 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Wed, 25 Sep 2019 01:20:56 -0700 Subject: [PATCH 2/5] promisor-remote.h: include missing header When we ran `make hdr-check`, we got the following warning message: promisor-remote.h:21:46: warning: declaration of 'struct repository' will not be visible outside of this function [-Wvisibility] extern int promisor_remote_get_direct(struct repository *repo, ^ 1 warning generated. This was caused by a missing reference to `struct repository`, which is defined in "repository.h". Include this missing header to fix this warning. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- promisor-remote.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/promisor-remote.h b/promisor-remote.h index 8200dfc940..76e8d86c7c 100644 --- a/promisor-remote.h +++ b/promisor-remote.h @@ -1,6 +1,8 @@ #ifndef PROMISOR_REMOTE_H #define PROMISOR_REMOTE_H +#include "repository.h" + struct object_id; /* From af26e2a9d265c7ad676e3b178c359a0ff75440a2 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Wed, 25 Sep 2019 01:20:59 -0700 Subject: [PATCH 3/5] pack-bitmap.h: remove magic number MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When we ran `make hdr-check` with the following patch diff --git a/Makefile b/Makefile index f879697ea3..d8df4e316b 100644 --- a/Makefile +++ b/Makefile @@ -2773,7 +2773,7 @@ CHK_HDRS = $(filter-out $(EXCEPT_HDRS),$(patsubst ./%,%,$(LIB_H))) HCO = $(patsubst %.h,%.hco,$(CHK_HDRS)) $(HCO): %.hco: %.h FORCE - $(QUIET_HDR)$(CC) -include git-compat-util.h -I. -o /dev/null -c -xc $< + $(QUIET_HDR)$(CC) -include git-compat-util.h -I. -o /dev/null -c -xc $(ALL_CFLAGS) $< .PHONY: hdr-check $(HCO) hdr-check: $(HCO) and with `DEVELOPER=1`, 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, thereby eliminating the magic number (4). An alternative was to simply add MAYBE_UNUSED, however that does not eliminate the magic number. Another alternative was to change the definition to extern const char BITMAP_IDX_SIGNATURE[4]; However, this design was also not chosen as the static definition allows us to keep the declaration together for readability along with removing the magic number. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- pack-bitmap.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pack-bitmap.h b/pack-bitmap.h index 00de3ec8e4..466c5afa09 100644 --- a/pack-bitmap.h +++ b/pack-bitmap.h @@ -9,16 +9,16 @@ struct commit; struct repository; struct rev_info; +static const char BITMAP_IDX_SIGNATURE[] = {'B', 'I', 'T', 'M'}; + struct bitmap_disk_header { - char magic[4]; + char magic[ARRAY_SIZE(BITMAP_IDX_SIGNATURE)]; uint16_t version; uint16_t options; uint32_t entry_count; unsigned char checksum[GIT_MAX_RAWSZ]; }; -static const char BITMAP_IDX_SIGNATURE[] = {'B', 'I', 'T', 'M'}; - #define NEEDS_BITMAP (1u<<22) enum pack_bitmap_opts { From b503a2d515e6ac3d2cced7881791c12663c45d01 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Wed, 25 Sep 2019 01:21:01 -0700 Subject: [PATCH 4/5] Makefile: emulate compile in $(HCO) target better Currently, when testing headers using `make hdr-check`, headers are directly compiled. Although this seems to test the headers, this is too strict since we treat the headers as C sources. As a result, this will cause warnings to appear that would otherwise not, such as a static variable definition intended for later use throwing a unused variable warning. In addition, on platforms that can run `make hdr-check` but require custom flags, this target was failing because none of them were being passed to the compiler. For example, on MacOS, the NO_OPENSSL flag was being set but it was not being passed into compiler so the check was failing. Fix these problems by emulating the compile process better, including test compiling dummy *.hcc C sources generated from the *.h files and passing $(ALL_CFLAGS) into the compiler for the $(HCO) target so that these custom flags can be used. Helped-by: Jeff King Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- .gitignore | 1 + Makefile | 12 +++++++++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/.gitignore b/.gitignore index 521d8f4fb4..34efe125cb 100644 --- a/.gitignore +++ b/.gitignore @@ -216,6 +216,7 @@ /tags /TAGS /cscope* +*.hcc *.obj *.lib *.res diff --git a/Makefile b/Makefile index f879697ea3..581cc617e3 100644 --- a/Makefile +++ b/Makefile @@ -1872,7 +1872,7 @@ ifndef V QUIET_MSGFMT = @echo ' ' MSGFMT $@; QUIET_GCOV = @echo ' ' GCOV $@; QUIET_SP = @echo ' ' SP $<; - QUIET_HDR = @echo ' ' HDR $<; + QUIET_HDR = @echo ' ' HDR $(<:hcc=h); QUIET_RC = @echo ' ' RC $@; QUIET_SUBDIR0 = +@subdir= QUIET_SUBDIR1 = ;$(NO_SUBDIR) echo ' ' SUBDIR $$subdir; \ @@ -2771,9 +2771,14 @@ ifndef GCRYPT_SHA256 endif CHK_HDRS = $(filter-out $(EXCEPT_HDRS),$(patsubst ./%,%,$(LIB_H))) HCO = $(patsubst %.h,%.hco,$(CHK_HDRS)) +HCC = $(HCO:hco=hcc) -$(HCO): %.hco: %.h FORCE - $(QUIET_HDR)$(CC) -include git-compat-util.h -I. -o /dev/null -c -xc $< +%.hcc: %.h + @echo '#include "git-compat-util.h"' >$@ + @echo '#include "$<"' >>$@ + +$(HCO): %.hco: %.hcc FORCE + $(QUIET_HDR)$(CC) $(ALL_CFLAGS) -o /dev/null -c -xc $< .PHONY: hdr-check $(HCO) hdr-check: $(HCO) @@ -3082,6 +3087,7 @@ clean: profile-clean coverage-clean cocciclean $(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) git$X $(RM) $(TEST_PROGRAMS) $(RM) $(FUZZ_PROGRAMS) + $(RM) $(HCC) $(RM) -r bin-wrappers $(dep_dirs) $(RM) -r po/build/ $(RM) *.pyc *.pyo */*.pyc */*.pyo command-list.h $(ETAGS_TARGET) tags cscope* From 411e4f4735397a601f33be475918e4a96f66e3a2 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 1 Oct 2019 04:16:26 -0700 Subject: [PATCH 5/5] ci: run `hdr-check` as part of the `Static Analysis` job Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- azure-pipelines.yml | 2 +- ci/install-dependencies.sh | 3 ++- ci/run-static-analysis.sh | 3 +++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index c329b7218b..1d4bcbda65 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -354,7 +354,7 @@ jobs: test "$GITFILESHAREPWD" = '$(gitfileshare.pwd)' || ci/mount-fileshare.sh //gitfileshare.file.core.windows.net/test-cache gitfileshare "$GITFILESHAREPWD" "$HOME/test-cache" || exit 1 sudo apt-get update && - sudo apt-get install -y coccinelle && + sudo apt-get install -y coccinelle libcurl4-openssl-dev libssl-dev libexpat-dev gettext && export jobname=StaticAnalysis && diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh index 8cc72503cb..8ce9ce276e 100755 --- a/ci/install-dependencies.sh +++ b/ci/install-dependencies.sh @@ -49,7 +49,8 @@ osx-clang|osx-gcc) ;; StaticAnalysis) sudo apt-get -q update - sudo apt-get -q -y install coccinelle + sudo apt-get -q -y install coccinelle libcurl4-openssl-dev libssl-dev \ + libexpat-dev gettext ;; Documentation) sudo apt-get -q update diff --git a/ci/run-static-analysis.sh b/ci/run-static-analysis.sh index a19aa7ebbc..65bcebda41 100755 --- a/ci/run-static-analysis.sh +++ b/ci/run-static-analysis.sh @@ -26,4 +26,7 @@ then exit 1 fi +make hdr-check || +exit 1 + save_good_tree