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 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



reply via email to

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