From d2bff22c2369289eac4d17654c464e392e726b3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Sat, 14 Apr 2018 19:19:43 +0000 Subject: connect.c: mark die_initial_contact() NORETURN MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There is a series running in parallel with this one that adds code like this switch (...) { case ...: die_initial_contact(); case ...: There is nothing wrong with this. There is no actual falling through. But since gcc is not that smart and gcc 7.x introduces -Wimplicit-fallthrough, it raises a false alarm in this case. This class of warnings may be useful elsewhere, so instead of suppressing the whole class, let's try to fix just this code. gcc is smart enough to realize that no execution can continue after a NORETURN function call and no longer raises the warning. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano diff --git a/connect.c b/connect.c index c3a014c..49eca46 100644 --- a/connect.c +++ b/connect.c @@ -46,7 +46,7 @@ int check_ref_type(const struct ref *ref, int flags) return check_ref(ref->name, flags); } -static void die_initial_contact(int unexpected) +static NORETURN void die_initial_contact(int unexpected) { if (unexpected) die(_("The remote end hung up upon initial contact")); -- cgit v0.10.2-6-g49f6 From 1da1580e4c2d482070363edc7b4f8b42874c312a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Sat, 14 Apr 2018 19:19:44 +0000 Subject: Makefile: detect compiler and enable more warnings in DEVELOPER=1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The set of extra warnings we enable when DEVELOPER has to be conservative because we can't assume any compiler version the developer may use. Detect the compiler version so we know when it's safe to enable -Wextra and maybe more. These warning settings are mostly from my custom config.mak a long time ago when I tried to enable as many warnings as possible that can still build without showing warnings. Some of those warnings are probably worth fixing instead of just suppressing in future. Helped-by: Jeff King Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano diff --git a/Makefile b/Makefile index 7f40f76..9ea2e50 100644 --- a/Makefile +++ b/Makefile @@ -431,6 +431,10 @@ all:: # # When cross-compiling, define HOST_CPU as the canonical name of the CPU on # which the built Git will run (for instance "x86_64"). +# +# Define DEVELOPER to enable more compiler warnings. Compiler version +# and family are auto detected, but could be overridden by defining +# COMPILER_FEATURES (see config.mak.dev) GIT-VERSION-FILE: FORCE @$(SHELL_PATH) ./GIT-VERSION-GEN @@ -439,15 +443,6 @@ GIT-VERSION-FILE: FORCE # CFLAGS and LDFLAGS are for the users to override from the command line. CFLAGS = -g -O2 -Wall -DEVELOPER_CFLAGS = -Werror \ - -Wdeclaration-after-statement \ - -Wno-format-zero-length \ - -Wold-style-definition \ - -Woverflow \ - -Wpointer-arith \ - -Wstrict-prototypes \ - -Wunused \ - -Wvla LDFLAGS = ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS) ALL_LDFLAGS = $(LDFLAGS) @@ -1047,7 +1042,7 @@ include config.mak.uname -include config.mak ifdef DEVELOPER -CFLAGS += $(DEVELOPER_CFLAGS) +include config.mak.dev endif comma := , diff --git a/config.mak.dev b/config.mak.dev new file mode 100644 index 0000000..716a14e --- /dev/null +++ b/config.mak.dev @@ -0,0 +1,38 @@ +CFLAGS += -Werror +CFLAGS += -Wdeclaration-after-statement +CFLAGS += -Wno-format-zero-length +CFLAGS += -Wold-style-definition +CFLAGS += -Woverflow +CFLAGS += -Wpointer-arith +CFLAGS += -Wstrict-prototypes +CFLAGS += -Wunused +CFLAGS += -Wvla + +ifndef COMPILER_FEATURES +COMPILER_FEATURES := $(shell ./detect-compiler $(CC)) +endif + +ifneq ($(filter clang4,$(COMPILER_FEATURES)),) +CFLAGS += -Wtautological-constant-out-of-range-compare +endif + +ifneq ($(or $(filter gcc6,$(COMPILER_FEATURES)),$(filter clang4,$(COMPILER_FEATURES))),) +CFLAGS += -Wextra +# if a function is public, there should be a prototype and the right +# header file should be included. If not, it should be static. +CFLAGS += -Wmissing-prototypes +# These are disabled because we have these all over the place. +CFLAGS += -Wno-empty-body +CFLAGS += -Wno-missing-field-initializers +CFLAGS += -Wno-sign-compare +CFLAGS += -Wno-unused-function +CFLAGS += -Wno-unused-parameter +endif + +# uninitialized warnings on gcc 4.9.2 in xdiff/xdiffi.c and config.c +# not worth fixing since newer compilers correctly stop complaining +ifneq ($(filter gcc4,$(COMPILER_FEATURES)),) +ifeq ($(filter gcc5,$(COMPILER_FEATURES)),) +CFLAGS += -Wno-uninitialized +endif +endif diff --git a/detect-compiler b/detect-compiler new file mode 100755 index 0000000..70b7544 --- /dev/null +++ b/detect-compiler @@ -0,0 +1,53 @@ +#!/bin/sh +# +# Probe the compiler for vintage, version, etc. This is used for setting +# optional make knobs under the DEVELOPER knob. + +CC="$*" + +# we get something like (this is at least true for gcc and clang) +# +# FreeBSD clang version 3.4.1 (tags/RELEASE...) +get_version_line() { + $CC -v 2>&1 | grep ' version ' +} + +get_family() { + get_version_line | sed 's/^\(.*\) version [0-9][^ ]* .*/\1/' +} + +get_version() { + get_version_line | sed 's/^.* version \([0-9][^ ]*\) .*/\1/' +} + +print_flags() { + family=$1 + version=$(get_version | cut -f 1 -d .) + + # Print a feature flag not only for the current version, but also + # for any prior versions we encompass. This avoids needing to do + # numeric comparisons in make, which are awkward. + while test "$version" -gt 0 + do + echo $family$version + version=$((version - 1)) + done +} + +case "$(get_family)" in +gcc) + print_flags gcc + ;; +clang) + print_flags clang + ;; +"FreeBSD clang") + print_flags clang + ;; +"Apple LLVM") + print_flags clang + ;; +*) + : unknown compiler family + ;; +esac -- cgit v0.10.2-6-g49f6 From 99f763baf5941455aefd176f9b1e98a4ff81f611 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Sat, 14 Apr 2018 19:19:45 +0000 Subject: Makefile: add a DEVOPTS to suppress -Werror under DEVELOPER MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a DEVOPTS variable that'll be used to tweak the behavior of DEVELOPER. I've long wanted to use DEVELOPER=1 in my production builds, but on some old systems I still get warnings, and thus the build would fail. However if the build/tests fail for some other reason, it would still be useful to scroll up and see what the relevant code is warning about. This change allows for that. Now setting DEVELOPER will set -Werror as before, but if DEVOPTS=no-error is provided is set you'll get the same warnings, but without -Werror. Helped-by: Nguyễn Thái Ngọc Duy Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano diff --git a/Makefile b/Makefile index 9ea2e50..f5fd5ac 100644 --- a/Makefile +++ b/Makefile @@ -435,6 +435,16 @@ all:: # Define DEVELOPER to enable more compiler warnings. Compiler version # and family are auto detected, but could be overridden by defining # COMPILER_FEATURES (see config.mak.dev) +# +# When DEVELOPER is set, DEVOPTS can be used to control compiler +# options. This variable contains keywords separated by +# whitespace. The following keywords are are recognized: +# +# no-error: +# +# suppresses the -Werror that implicitly comes with +# DEVELOPER=1. Useful for getting the full set of errors +# without immediately dying, or for logging them. GIT-VERSION-FILE: FORCE @$(SHELL_PATH) ./GIT-VERSION-GEN diff --git a/config.mak.dev b/config.mak.dev index 716a14e..7a54426 100644 --- a/config.mak.dev +++ b/config.mak.dev @@ -1,4 +1,6 @@ +ifeq ($(filter no-error,$(DEVOPTS)),) CFLAGS += -Werror +endif CFLAGS += -Wdeclaration-after-statement CFLAGS += -Wno-format-zero-length CFLAGS += -Wold-style-definition -- cgit v0.10.2-6-g49f6 From 26d2e4fb227c1415011a136fa9bb881ee338118a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Sat, 14 Apr 2018 19:19:46 +0000 Subject: Makefile: add a DEVOPTS to get all of -Wextra MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change DEVOPTS to understand a "extra-all" option. When the DEVELOPER flag is enabled we turn on -Wextra, but manually switch some of the warnings it turns on off. This is because we have many existing occurrences of them in the code base. This mode will stop the suppression, let the developer see and decide whether to fix them. This change is a slight alteration of Nguyễn Thái Ngọc Duy EAGER_DEVELOPER mode patch[1] 1. "[PATCH v3 3/3] Makefile: add EAGER_DEVELOPER mode" (<20180329150322.10722-4-pclouds@gmail.com>; https://public-inbox.org/git/20180329150322.10722-4-pclouds@gmail.com/) Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano diff --git a/Makefile b/Makefile index f5fd5ac..1ad1049 100644 --- a/Makefile +++ b/Makefile @@ -445,6 +445,12 @@ all:: # suppresses the -Werror that implicitly comes with # DEVELOPER=1. Useful for getting the full set of errors # without immediately dying, or for logging them. +# +# extra-all: +# +# The DEVELOPER mode enables -Wextra with a few exceptions. By +# setting this flag the exceptions are removed, and all of +# -Wextra is used. GIT-VERSION-FILE: FORCE @$(SHELL_PATH) ./GIT-VERSION-GEN diff --git a/config.mak.dev b/config.mak.dev index 7a54426..2d244ca 100644 --- a/config.mak.dev +++ b/config.mak.dev @@ -23,6 +23,7 @@ CFLAGS += -Wextra # if a function is public, there should be a prototype and the right # header file should be included. If not, it should be static. CFLAGS += -Wmissing-prototypes +ifeq ($(filter extra-all,$(DEVOPTS)),) # These are disabled because we have these all over the place. CFLAGS += -Wno-empty-body CFLAGS += -Wno-missing-field-initializers @@ -30,6 +31,7 @@ CFLAGS += -Wno-sign-compare CFLAGS += -Wno-unused-function CFLAGS += -Wno-unused-parameter endif +endif # uninitialized warnings on gcc 4.9.2 in xdiff/xdiffi.c and config.c # not worth fixing since newer compilers correctly stop complaining -- cgit v0.10.2-6-g49f6