[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 Kiper |
Subject: |
Re: [PATCH v3 2/5] configure: Set gnu99 C language standard by default |
Date: |
Mon, 18 May 2020 17:49:52 +0200 |
User-agent: |
NeoMutt/20170113 (1.7.2) |
Hi Daniel,
Adding Patrick...
On Fri, May 15, 2020 at 11:56:56AM +1000, Daniel Axtens wrote:
> 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.
Thank you for testing this...
> It does not,
Great!
> 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.
Patrick, could you take a look at this?
> 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.
Javier, could you take a look at this too?
> 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.
Yeah, I saw that too. However, I think we have to enforce C standard in
one way or another. So, I would not bother much if we chose gnu99.
Daniel
[PATCH v3 3/5] INSTALL/configure: Update install doc and configure comment, Daniel Kiper, 2020/05/13
Re: [PATCH v3 0/5] Various build and doc fixes, Leif Lindholm, 2020/05/13