[libvirt] [PATCH] build: ask for -std=gnu99 explicitly

Daniel P. Berrangé posted 1 patch 1 week ago
Test syntax-check passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20191007140952.18753-1-berrange@redhat.com
m4/virt-compile-warnings.m4 | 4 ++++
1 file changed, 4 insertions(+)

[libvirt] [PATCH] build: ask for -std=gnu99 explicitly

Posted by Daniel P. Berrangé 1 week ago
We previously got -std=gnu99 secretly enabled as a side-effect
of requesting the 'stdarg' gnulib module. We rely on some
extensions from c99/gnu99 and while RHEL-7 supports this, it
still defaults to gnu89.  RHEL-7 also supports some newer
standards but declares them experimental/incomplete, so sticking
with gnu99 is best bet for now & matches historical usage.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 m4/virt-compile-warnings.m4 | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4
index f9460e82ba..502f7384ff 100644
--- a/m4/virt-compile-warnings.m4
+++ b/m4/virt-compile-warnings.m4
@@ -189,6 +189,10 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[
       wantwarn="$wantwarn -Werror"
     fi
 
+    # Request the gnu99 standard which is the best choice with
+    # gcc 4.8.0
+    wantwarn="-std=gnu99"
+
     # Check for $CC support of each warning
     for w in $wantwarn; do
       gl_WARN_ADD([$w])
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] build: ask for -std=gnu99 explicitly

Posted by Peter Krempa 1 week ago
On Mon, Oct 07, 2019 at 15:09:52 +0100, Daniel Berrange wrote:
> We previously got -std=gnu99 secretly enabled as a side-effect
> of requesting the 'stdarg' gnulib module. We rely on some
> extensions from c99/gnu99 and while RHEL-7 supports this, it
> still defaults to gnu89.  RHEL-7 also supports some newer
> standards but declares them experimental/incomplete, so sticking
> with gnu99 is best bet for now & matches historical usage.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  m4/virt-compile-warnings.m4 | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4
> index f9460e82ba..502f7384ff 100644
> --- a/m4/virt-compile-warnings.m4
> +++ b/m4/virt-compile-warnings.m4
> @@ -189,6 +189,10 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[
>        wantwarn="$wantwarn -Werror"
>      fi
>  
> +    # Request the gnu99 standard which is the best choice with
> +    # gcc 4.8.0
> +    wantwarn="-std=gnu99"
> +

It feels odd to add this via the warning flags variable.

I know this is a hack until we get rid of the old build system, but I
think you should at least admit it in the commit message or comment that
it's deliberate.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] build: ask for -std=gnu99 explicitly

Posted by Daniel P. Berrangé 1 week ago
On Mon, Oct 07, 2019 at 04:27:31PM +0200, Peter Krempa wrote:
> On Mon, Oct 07, 2019 at 15:09:52 +0100, Daniel Berrange wrote:
> > We previously got -std=gnu99 secretly enabled as a side-effect
> > of requesting the 'stdarg' gnulib module. We rely on some
> > extensions from c99/gnu99 and while RHEL-7 supports this, it
> > still defaults to gnu89.  RHEL-7 also supports some newer
> > standards but declares them experimental/incomplete, so sticking
> > with gnu99 is best bet for now & matches historical usage.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  m4/virt-compile-warnings.m4 | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4
> > index f9460e82ba..502f7384ff 100644
> > --- a/m4/virt-compile-warnings.m4
> > +++ b/m4/virt-compile-warnings.m4
> > @@ -189,6 +189,10 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[
> >        wantwarn="$wantwarn -Werror"
> >      fi
> >  
> > +    # Request the gnu99 standard which is the best choice with
> > +    # gcc 4.8.0
> > +    wantwarn="-std=gnu99"
> > +
> 
> It feels odd to add this via the warning flags variable.
> 
> I know this is a hack until we get rid of the old build system, but I
> think you should at least admit it in the commit message or comment that
> it's deliberate.

It is nothing new, we've long used this file for things that are not mere
warning flags, eg setting FORTIFY_SOURCE or -fstack-protector. Really
its just a bad filename, but its not worth worrying about that at this
point IMHO.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] build: ask for -std=gnu99 explicitly

Posted by Peter Krempa 1 week ago
On Mon, Oct 07, 2019 at 15:52:07 +0100, Daniel Berrange wrote:
> On Mon, Oct 07, 2019 at 04:27:31PM +0200, Peter Krempa wrote:
> > On Mon, Oct 07, 2019 at 15:09:52 +0100, Daniel Berrange wrote:
> > > We previously got -std=gnu99 secretly enabled as a side-effect
> > > of requesting the 'stdarg' gnulib module. We rely on some
> > > extensions from c99/gnu99 and while RHEL-7 supports this, it
> > > still defaults to gnu89.  RHEL-7 also supports some newer
> > > standards but declares them experimental/incomplete, so sticking
> > > with gnu99 is best bet for now & matches historical usage.
> > > 
> > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > > ---
> > >  m4/virt-compile-warnings.m4 | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4
> > > index f9460e82ba..502f7384ff 100644
> > > --- a/m4/virt-compile-warnings.m4
> > > +++ b/m4/virt-compile-warnings.m4
> > > @@ -189,6 +189,10 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[
> > >        wantwarn="$wantwarn -Werror"
> > >      fi
> > >  
> > > +    # Request the gnu99 standard which is the best choice with
> > > +    # gcc 4.8.0
> > > +    wantwarn="-std=gnu99"
> > > +
> > 
> > It feels odd to add this via the warning flags variable.
> > 
> > I know this is a hack until we get rid of the old build system, but I
> > think you should at least admit it in the commit message or comment that
> > it's deliberate.
> 
> It is nothing new, we've long used this file for things that are not mere
> warning flags, eg setting FORTIFY_SOURCE or -fstack-protector. Really
> its just a bad filename, but its not worth worrying about that at this
> point IMHO.

Eww.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] build: ask for -std=gnu99 explicitly

Posted by Fabiano Fidêncio 1 week ago
On Mon, Oct 7, 2019 at 4:16 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> We previously got -std=gnu99 secretly enabled as a side-effect
> of requesting the 'stdarg' gnulib module. We rely on some
> extensions from c99/gnu99 and while RHEL-7 supports this, it
> still defaults to gnu89.  RHEL-7 also supports some newer
> standards but declares them experimental/incomplete, so sticking
> with gnu99 is best bet for now & matches historical usage.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  m4/virt-compile-warnings.m4 | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4
> index f9460e82ba..502f7384ff 100644
> --- a/m4/virt-compile-warnings.m4
> +++ b/m4/virt-compile-warnings.m4
> @@ -189,6 +189,10 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[
>        wantwarn="$wantwarn -Werror"
>      fi
>
> +    # Request the gnu99 standard which is the best choice with
> +    # gcc 4.8.0
> +    wantwarn="-std=gnu99"
> +
>      # Check for $CC support of each warning
>      for w in $wantwarn; do
>        gl_WARN_ADD([$w])
> --
> 2.21.0
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list