grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 2/5] configure: Set gnu99 C language standard by default


From: Daniel Axtens
Subject: Re: [PATCH v3 2/5] configure: Set gnu99 C language standard by default
Date: Fri, 15 May 2020 11:56:56 +1000

Hi Daniel,

> Commit d5a32255d (misc: Make grub_strtol() "end" pointers have safer
> const qualifiers) introduced "restrict" keyword into some functions
> definitions. This keyword was introduced in C99 standard. However, some
> compilers by default may use C89 or something different. This behavior
> leads to the breakage during builds when c89 or gnu89 is in force. So,
> let's set gnu99 C language standard for all compilers by default. This
> way a bit random build issue will be fixed and the GRUB source will be
> build consistently regardless of type and version of the compiler.
>
> It was decided to use gnu99 C language standard because it fixes the
> issue mentioned above and also provides some useful extensions which are
> used here and there in the GRUB source. Potentially we can use gnu11
> too. However, this may reduce pool of older compilers which can be used
> to build the GRUB. So, let's live with gnu99 until we discover that we
> strongly require a feature from newer C standard.

I tried building this with CC=clang just to see if it didn't like gnu99
for some reason.

It does not, but for different reasons to what I expected:

clang -DHAVE_CONFIG_H -I.  -Wall -W -DGRUB_UTIL=1 -D_FILE_OFFSET_BITS=64 
-I./include -DGRUB_FILE=\"grub-core/lib/json/json.c\" -I. -I. -I. -I. 
-I./include -I./include -I./grub-core/lib/libgcrypt-grub/src/  
-I./grub-core/lib/gnulib -I./grub-core/lib/gnulib -I./grub-core/lib/json 
-D_FILE_OFFSET_BITS=64 -std=gnu99  -Wall -W -Wshadow -Wpointer-arith -Wundef 
-Wchar-subscripts -Wcomment -Wdeprecated-declarations -Wdisabled-optimization 
-Wdiv-by-zero -Wfloat-equal -Wformat-extra-args -Wformat-security -Wformat-y2k 
-Wimplicit -Wimplicit-function-declaration -Wimplicit-int -Wmain 
-Wmissing-braces -Wmissing-format-attribute -Wmultichar -Wparentheses 
-Wreturn-type -Wsequence-point -Wshadow -Wsign-compare -Wswitch -Wtrigraphs 
-Wunknown-pragmas -Wunused -Wunused-function -Wunused-label -Wunused-parameter 
-Wunused-value  -Wunused-variable -Wwrite-strings -Wnested-externs 
-Wstrict-prototypes -Wcast-align  -Wextra -Wattributes -Wendif-labels 
-Winit-self -Wint-to-pointer-cast -Winvalid-pch -Wmissing-field-initializers 
-Wnonnull -Woverflow -Wvla -Wpointer-to-int-cast -Wstrict-aliasing 
-Wvariadic-macros -Wvolatile-register-var -Wpointer-sign -Wmissing-include-dirs 
-Wmissing-prototypes -Wmissing-declarations -Wformat=2 -Werror  -Wno-undef 
-Wno-sign-compare -Wno-unused -Wno-unused-parameter -Wno-redundant-decls 
-Wno-unreachable-code -Wno-conversion  -MT 
grub-core/lib/json/libgrubkern_a-json.o -MD -MP -MF 
grub-core/lib/json/.deps-util/libgrubkern_a-json.Tpo -c -o 
grub-core/lib/json/libgrubkern_a-json.o `test -f 'grub-core/lib/json/json.c' || 
echo './'`grub-core/lib/json/json.c
In file included from grub-core/lib/json/json.c:24:
./grub-core/lib/json/json.h:39:24: error: redefinition of typedef 'jsmntok_t' 
is a C11 feature [-Werror,-Wtypedef-redefinition]
typedef struct jsmntok jsmntok_t;
                       ^
./grub-core/lib/json/jsmn.h:77:3: note: previous definition is here
} jsmntok_t;
  ^
1 error generated.


clang --version
clang version 9.0.0-2 (tags/RELEASE_900/final)
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

Without this patch, clang-9 fails on the use of nested functions in
commit cb2f15c54489 ("normal/main: Search for specific config files for
netboot") - I'll send a fix for that shortly. If I revert that as a
temporary measure, clang will build.

I notice from ./configure that it already does some form of testing for
restrict:

checking for C/C++ restrict keyword... __restrict

I'm not sure what brings that test in to configure (gnulib?), and
accessing it is a little bit tricky because we use AC_CONFIG_FILES for
config.h rather than AC_CONFIG_HEADER, but that could be a general
purpose solution.

Regards,
Daniel

> The user is still able to override C language standard using relevant
> *_CFLAGS variables.
>
> Signed-off-by: Daniel Kiper <address@hidden>
> ---
> v3 - suggestions/fixes:
>    - add a comment before the change in configure.ac
>      (suggested by Javier Martinez Canillas and Leif Lindholm),
>    - improve commit message
>      (suggested by Javier Martinez Canillas and Leif Lindholm).
>
> v2 - suggestions/fixes:
>    - unconditionally enforce gnu99 C language standard
>      (suggested by Leif Lindholm).
> ---
>  configure.ac | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/configure.ac b/configure.ac
> index b2576b013..3eda9a5a2 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -80,6 +80,12 @@ if test "x$TARGET_CFLAGS" = x; then
>    TARGET_CFLAGS=-Os
>  fi
>  
> +# Enable support for "restrict" keyword and other
> +# features from gnu99 C language standard.
> +BUILD_CFLAGS="-std=gnu99 $BUILD_CFLAGS"
> +HOST_CFLAGS="-std=gnu99 $HOST_CFLAGS"
> +TARGET_CFLAGS="-std=gnu99 $TARGET_CFLAGS"
> +
>  # Default HOST_CPPFLAGS
>  HOST_CPPFLAGS="$HOST_CPPFLAGS -Wall -W"
>  HOST_CPPFLAGS="$HOST_CPPFLAGS -DGRUB_UTIL=1"
> -- 
> 2.11.0
>
>
> _______________________________________________
> Grub-devel mailing list
> address@hidden
> https://lists.gnu.org/mailman/listinfo/grub-devel



reply via email to

[Prev in Thread] Current Thread [Next in Thread]