[PATCH] meson: Check for stdarg.h

Michal Privoznik posted 1 patch 3 years, 6 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/94293ff3925b707cad3225a54529db7b843b43c1.1599048474.git.mprivozn@redhat.com
meson.build | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
[PATCH] meson: Check for stdarg.h
Posted by Michal Privoznik 3 years, 6 months ago
As it turns out, one of my previous commits in which I removed
checking for stdarg.h was too aggressive. Long story short, the
readline public headers rely on stdarg.h and what is worse, they
expect us to declare the autotools style of macro (HAVE_STDARG_H)
if the header file exists. If we don't do it then compiling virsh
on macos fails.

See 9ea3424a178 for more info.

Fixes: 85808b73846f93d656b4c81b6ebddd2dc3881bf6
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 meson.build | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/meson.build b/meson.build
index 1aad385ad1..98f7545495 100644
--- a/meson.build
+++ b/meson.build
@@ -1333,8 +1333,12 @@ if readline_dep.found()
     endif
   endif
 
-  # We need this to avoid compilation issues with modern compilers.
-  # See 9ea3424a178 for a more detailed explanation
+  # We need both of these hacks to avoid compilation issues with modern
+  # compilers. See 9ea3424a178 for a more detailed explanation.
+  if cc.has_header('stdarg.h')
+    conf.set('HAVE_STDARG_H', 1)
+  endif
+
   readline_dep = declare_dependency(
     compile_args: [ '-D_FUNCTION_DEF' ],
     dependencies: [ readline_dep ],
-- 
2.26.2

Re: [PATCH] meson: Check for stdarg.h
Posted by Daniel P. Berrangé 3 years, 6 months ago
On Wed, Sep 02, 2020 at 02:07:54PM +0200, Michal Privoznik wrote:
> As it turns out, one of my previous commits in which I removed
> checking for stdarg.h was too aggressive. Long story short, the
> readline public headers rely on stdarg.h and what is worse, they
> expect us to declare the autotools style of macro (HAVE_STDARG_H)
> if the header file exists. If we don't do it then compiling virsh
> on macos fails.
> 
> See 9ea3424a178 for more info.

Ewww....

Deprecated in 2000,  removed in 2013, then immediately readded to
"fix" apps which still relied on K&R C with no function prototypes.

The readline maintainer is more forgiving of ancient application code
than I would be :-) 30 years since arrival of ANSI C is enough time
to update code to use function prototypes, especially if you want to
build against a readline library released in 2020, as opposed to the
old version released years ago.

> 
> Fixes: 85808b73846f93d656b4c81b6ebddd2dc3881bf6
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  meson.build | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/meson.build b/meson.build
> index 1aad385ad1..98f7545495 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1333,8 +1333,12 @@ if readline_dep.found()
>      endif
>    endif
>  
> -  # We need this to avoid compilation issues with modern compilers.
> -  # See 9ea3424a178 for a more detailed explanation
> +  # We need both of these hacks to avoid compilation issues with modern
> +  # compilers. See 9ea3424a178 for a more detailed explanation.
> +  if cc.has_header('stdarg.h')
> +    conf.set('HAVE_STDARG_H', 1)
> +  endif

Do we have any platforms which lack stdarg.h ? eg can be just add
"#define HAVE_STDARG_H 1" unconditionally in the virsh code before
it includes the readline headers ?

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

Re: [PATCH] meson: Check for stdarg.h
Posted by Michal Privoznik 3 years, 6 months ago
On 9/2/20 2:20 PM, Daniel P. Berrangé wrote:
> On Wed, Sep 02, 2020 at 02:07:54PM +0200, Michal Privoznik wrote:
>> As it turns out, one of my previous commits in which I removed
>> checking for stdarg.h was too aggressive. Long story short, the
>> readline public headers rely on stdarg.h and what is worse, they
>> expect us to declare the autotools style of macro (HAVE_STDARG_H)
>> if the header file exists. If we don't do it then compiling virsh
>> on macos fails.
>>
>> See 9ea3424a178 for more info.
> 
> Ewww....
> 
> Deprecated in 2000,  removed in 2013, then immediately readded to
> "fix" apps which still relied on K&R C with no function prototypes.
> 
> The readline maintainer is more forgiving of ancient application code
> than I would be :-) 30 years since arrival of ANSI C is enough time
> to update code to use function prototypes, especially if you want to
> build against a readline library released in 2020, as opposed to the
> old version released years ago.
> 
>>
>> Fixes: 85808b73846f93d656b4c81b6ebddd2dc3881bf6
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>>   meson.build | 8 ++++++--
>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/meson.build b/meson.build
>> index 1aad385ad1..98f7545495 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -1333,8 +1333,12 @@ if readline_dep.found()
>>       endif
>>     endif
>>   
>> -  # We need this to avoid compilation issues with modern compilers.
>> -  # See 9ea3424a178 for a more detailed explanation
>> +  # We need both of these hacks to avoid compilation issues with modern
>> +  # compilers. See 9ea3424a178 for a more detailed explanation.
>> +  if cc.has_header('stdarg.h')
>> +    conf.set('HAVE_STDARG_H', 1)
>> +  endif
> 
> Do we have any platforms which lack stdarg.h ? eg can be just add
> "#define HAVE_STDARG_H 1" unconditionally in the virsh code before
> it includes the readline headers ?

Looks like uclibc doesn't provide stdarg.h but do we even support it? On 
the other hand, even musl provides the header file. So I guess what you 
suggests will do.

Michal

Re: [PATCH] meson: Check for stdarg.h
Posted by Ján Tomko 3 years, 6 months ago
On a Wednesday in 2020, Daniel P. Berrangé wrote:
>On Wed, Sep 02, 2020 at 02:07:54PM +0200, Michal Privoznik wrote:
>> As it turns out, one of my previous commits in which I removed
>> checking for stdarg.h was too aggressive. Long story short, the
>> readline public headers rely on stdarg.h and what is worse, they
>> expect us to declare the autotools style of macro (HAVE_STDARG_H)
>> if the header file exists. If we don't do it then compiling virsh
>> on macos fails.
>>
>> See 9ea3424a178 for more info.
>
>Ewww....
>
>Deprecated in 2000,  removed in 2013, then immediately readded to
>"fix" apps which still relied on K&R C with no function prototypes.
>
>The readline maintainer is more forgiving of ancient application code
>than I would be :-) 30 years since arrival of ANSI C is enough time
>to update code to use function prototypes, especially if you want to
>build against a readline library released in 2020, as opposed to the
>old version released years ago.
>
>>
>> Fixes: 85808b73846f93d656b4c81b6ebddd2dc3881bf6
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>>  meson.build | 8 ++++++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/meson.build b/meson.build
>> index 1aad385ad1..98f7545495 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -1333,8 +1333,12 @@ if readline_dep.found()
>>      endif
>>    endif
>>
>> -  # We need this to avoid compilation issues with modern compilers.
>> -  # See 9ea3424a178 for a more detailed explanation
>> +  # We need both of these hacks to avoid compilation issues with modern
>> +  # compilers. See 9ea3424a178 for a more detailed explanation.
>> +  if cc.has_header('stdarg.h')
>> +    conf.set('HAVE_STDARG_H', 1)
>> +  endif
>
>Do we have any platforms which lack stdarg.h ? eg can be just add
>"#define HAVE_STDARG_H 1" unconditionally in the virsh code before
>it includes the readline headers ?
>

It's part of the C99 standard:
https://pubs.opengroup.org/onlinepubs/009695399/basedefs/stdarg.h.html

(but I'm still confused why this builds on other platforms even though
  HAVE_STDARG_H is not defined anywhere)

Jano

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

Re: [PATCH] meson: Check for stdarg.h
Posted by Roman Bogorodskiy 3 years, 6 months ago
  Ján Tomko wrote:

> On a Wednesday in 2020, Daniel P. Berrangé wrote:
> >On Wed, Sep 02, 2020 at 02:07:54PM +0200, Michal Privoznik wrote:
> >> As it turns out, one of my previous commits in which I removed
> >> checking for stdarg.h was too aggressive. Long story short, the
> >> readline public headers rely on stdarg.h and what is worse, they
> >> expect us to declare the autotools style of macro (HAVE_STDARG_H)
> >> if the header file exists. If we don't do it then compiling virsh
> >> on macos fails.
> >>
> >> See 9ea3424a178 for more info.
> >
> >Ewww....
> >
> >Deprecated in 2000,  removed in 2013, then immediately readded to
> >"fix" apps which still relied on K&R C with no function prototypes.
> >
> >The readline maintainer is more forgiving of ancient application code
> >than I would be :-) 30 years since arrival of ANSI C is enough time
> >to update code to use function prototypes, especially if you want to
> >build against a readline library released in 2020, as opposed to the
> >old version released years ago.
> >
> >>
> >> Fixes: 85808b73846f93d656b4c81b6ebddd2dc3881bf6
> >> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> >> ---
> >>  meson.build | 8 ++++++--
> >>  1 file changed, 6 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/meson.build b/meson.build
> >> index 1aad385ad1..98f7545495 100644
> >> --- a/meson.build
> >> +++ b/meson.build
> >> @@ -1333,8 +1333,12 @@ if readline_dep.found()
> >>      endif
> >>    endif
> >>
> >> -  # We need this to avoid compilation issues with modern compilers.
> >> -  # See 9ea3424a178 for a more detailed explanation
> >> +  # We need both of these hacks to avoid compilation issues with modern
> >> +  # compilers. See 9ea3424a178 for a more detailed explanation.
> >> +  if cc.has_header('stdarg.h')
> >> +    conf.set('HAVE_STDARG_H', 1)
> >> +  endif
> >
> >Do we have any platforms which lack stdarg.h ? eg can be just add
> >"#define HAVE_STDARG_H 1" unconditionally in the virsh code before
> >it includes the readline headers ?
> >
> 
> It's part of the C99 standard:
> https://pubs.opengroup.org/onlinepubs/009695399/basedefs/stdarg.h.html
> 
> (but I'm still confused why this builds on other platforms even though
>   HAVE_STDARG_H is not defined anywhere)
> 
> Jano
> 

It fails to build on FreeBSD as well:

In file included from ../tools/vsh.c:33:
/usr/local/include/readline/readline.h:398:23: error: this function declaration is not a prototype [-Werror,-Wstrict-prototypes]
extern int rl_message ();
                      ^
                       void
1 error generated.


Roman Bogorodskiy