Re: [PATCH 1/1] git-compat-util: add a test balloon for C99 support
flat view
From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2021-11-15 01:24:27
Subsystem:
kernel build + files below scripts/ (unless maintained elsewhere), the rest · Maintainers:
Nathan Chancellor, Nicolas Schier, Linus Torvalds
On Sun, Nov 14 2021, brian m. carlson wrote:
The C99 standard was released in January 1999, now 22 years ago. It provides a variety of useful features, including variadic arguments for macros, declarations after statements, variable length arrays, and a wide variety of other useful features, many of which we already use. We'd like to take advantage of these features, but we want to be cautious. As far as we know, all major compilers now support C99 or a later C standard, such as C11 or C17. POSIX has required C99 support as a requirement for the 2001 revision, so we can safely assume any POSIX system which we are interested in supporting has C99.
I like this direction.
Sparse is also updated with a reference to the gnu99 standard, without which it defaults to C89.
Do we really need it in SPARSE_FLAGS though...
quoted hunk ↗ jump to hunk
@@ -1204,7 +1204,7 @@ endif # Set CFLAGS, LDFLAGS and other *FLAGS variables. These might be # tweaked by config.* below as well as the command-line, both of # which'll override these defaults. -CFLAGS = -g -O2 -Wall +CFLAGS = -g -O2 -Wall -std=gnu99 LDFLAGS = CC_LD_DYNPATH = -Wl,-rpath, BASIC_CFLAGS = -I.@@ -1215,7 +1215,7 @@ ARFLAGS = rcs PTHREAD_CFLAGS =
Since $(CFLAGS) ends up in:
ALL_CFLAGS = $(DEVELOPER_CFLAGS) $(CPPFLAGS) $(CFLAGS)
...
quoted hunk ↗ jump to hunk
# For the 'sparse' target -SPARSE_FLAGS ?= +SPARSE_FLAGS ?= -std=gnu99 SP_EXTRA_FLAGS = -Wno-universal-initializer
... and this will be used for this rule:
$(SP_OBJ): %.sp: %.c %.o
$(QUIET_SP)cgcc -no-compile $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) \
-Wsparse-error \
$(SPARSE_FLAGS) $(SP_EXTRA_FLAGS) $< [...]
I.e. unless it needs to be later on the command-line the $(ALL_CFLAGS)
should put it there already.
Also (and this pre-dates this patch) it's unfortunate that CFLAGS is a
mixed bag of compiler tweaking and "mandatory" flags. I think the below
would be a better approach, particurly since our own config.mak.uname
will override CFLAGS in some cases, and probably everyone who works on
git to any degree has a local config.mak which sets it to something
already.
But why gnu99 and not c99?
diff --git a/Makefile b/Makefile
index 12be39ac497..7470d627165 100644
--- a/Makefile
+++ b/Makefile@@ -1204,10 +1204,14 @@ endif # Set CFLAGS, LDFLAGS and other *FLAGS variables. These might be # tweaked by config.* below as well as the command-line, both of # which'll override these defaults. +# +# The MANDATORY_CFLAGS can be similarly overridden, but really +# shouldn't. CFLAGS = -g -O2 -Wall LDFLAGS = CC_LD_DYNPATH = -Wl,-rpath, -BASIC_CFLAGS = -I. +BASIC_CFLAGS = +MANDATORY_CFLAGS = -I. -std=gnu99 BASIC_LDFLAGS = # library flags
@@ -1249,7 +1253,7 @@ ALL_COMMANDS_TO_INSTALL += git-upload-archive$(X) ALL_COMMANDS_TO_INSTALL += git-upload-pack$(X) endif -ALL_CFLAGS = $(DEVELOPER_CFLAGS) $(CPPFLAGS) $(CFLAGS) +ALL_CFLAGS = $(DEVELOPER_CFLAGS) $(CPPFLAGS) $(CFLAGS) $(MANDATORY_CFLAGS) ALL_LDFLAGS = $(LDFLAGS) comma := ,