[PATCH v3 4/9] osdep.h: Always include <sys/signal.h> if it exists

Peter Maydell posted 9 patches 5 years, 3 months ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Paul Durrant <paul@xen.org>, Anthony Perard <anthony.perard@citrix.com>, Stefano Stabellini <sstabellini@kernel.org>
[PATCH v3 4/9] osdep.h: Always include <sys/signal.h> if it exists
Posted by Peter Maydell 5 years, 3 months ago
From: David CARLIER <devnexen@gmail.com>

Regularize our handling of <sys/signal.h>: currently we include it in
osdep.h, but only for OpenBSD, and we include it without an ifdef
guard in a couple of C files.  This causes problems for Haiku, which
doesn't have that header.

Instead, check in configure whether sys/signal.h exists, and if it
does then always include it from osdep.h.

Signed-off-by: David Carlier <devnexen@gmail.com>
[PMM: Expanded commit message]
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 configure                   | 8 ++++++++
 include/qemu/osdep.h        | 2 +-
 hw/xen/xen-legacy-backend.c | 1 -
 util/oslib-posix.c          | 1 -
 4 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index ddc53d873ef..d131f760d8f 100755
--- a/configure
+++ b/configure
@@ -3212,6 +3212,11 @@ if ! check_include "ifaddrs.h" ; then
   have_ifaddrs_h=no
 fi
 
+have_sys_signal_h=no
+if check_include "sys/signal.h" ; then
+  have_sys_signal_h=yes
+fi
+
 ##########################################
 # VTE probe
 
@@ -7398,6 +7403,9 @@ fi
 if test "$have_broken_size_max" = "yes" ; then
     echo "HAVE_BROKEN_SIZE_MAX=y" >> $config_host_mak
 fi
+if test "$have_sys_signal_h" = "yes" ; then
+    echo "CONFIG_SYS_SIGNAL=y" >> $config_host_mak
+fi
 
 # Work around a system header bug with some kernel/XFS header
 # versions where they both try to define 'struct fsxattr':
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 0d26a1b9bd0..6e0cf9132d9 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -104,7 +104,7 @@ extern int daemon(int, int);
 #include <setjmp.h>
 #include <signal.h>
 
-#ifdef __OpenBSD__
+#ifdef CONFIG_SYS_SIGNAL
 #include <sys/signal.h>
 #endif
 
diff --git a/hw/xen/xen-legacy-backend.c b/hw/xen/xen-legacy-backend.c
index 7d4b13351e0..965abe3ad34 100644
--- a/hw/xen/xen-legacy-backend.c
+++ b/hw/xen/xen-legacy-backend.c
@@ -23,7 +23,6 @@
  */
 
 #include "qemu/osdep.h"
-#include <sys/signal.h>
 
 #include "hw/sysbus.h"
 #include "hw/boards.h"
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 39ddc77c85b..7ad9195c445 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -38,7 +38,6 @@
 #include "qemu/sockets.h"
 #include "qemu/thread.h"
 #include <libgen.h>
-#include <sys/signal.h>
 #include "qemu/cutils.h"
 
 #ifdef CONFIG_LINUX
-- 
2.20.1


Re: [PATCH v3 4/9] osdep.h: Always include <sys/signal.h> if it exists
Posted by Thomas Huth 5 years, 3 months ago
On 03/07/2020 16.56, Peter Maydell wrote:
> From: David CARLIER <devnexen@gmail.com>
> 
> Regularize our handling of <sys/signal.h>: currently we include it in
> osdep.h, but only for OpenBSD, and we include it without an ifdef
> guard in a couple of C files.  This causes problems for Haiku, which
> doesn't have that header.
> 
> Instead, check in configure whether sys/signal.h exists, and if it
> does then always include it from osdep.h.
> 
> Signed-off-by: David Carlier <devnexen@gmail.com>
> [PMM: Expanded commit message]
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  configure                   | 8 ++++++++
>  include/qemu/osdep.h        | 2 +-
>  hw/xen/xen-legacy-backend.c | 1 -
>  util/oslib-posix.c          | 1 -
>  4 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/configure b/configure
> index ddc53d873ef..d131f760d8f 100755
> --- a/configure
> +++ b/configure
> @@ -3212,6 +3212,11 @@ if ! check_include "ifaddrs.h" ; then
>    have_ifaddrs_h=no
>  fi
>  
> +have_sys_signal_h=no
> +if check_include "sys/signal.h" ; then
> +  have_sys_signal_h=yes
> +fi
> +
>  ##########################################
>  # VTE probe
>  
> @@ -7398,6 +7403,9 @@ fi
>  if test "$have_broken_size_max" = "yes" ; then
>      echo "HAVE_BROKEN_SIZE_MAX=y" >> $config_host_mak
>  fi
> +if test "$have_sys_signal_h" = "yes" ; then
> +    echo "CONFIG_SYS_SIGNAL=y" >> $config_host_mak
> +fi

I'd maybe rather name it HAVE_SYS_SIGNAL_H, but I guess that's just a
matter of taste.

Reviewed-by: Thomas Huth <thuth@redhat.com>


Re: [PATCH v3 4/9] osdep.h: Always include <sys/signal.h> if it exists
Posted by Philippe Mathieu-Daudé 5 years, 3 months ago
On 7/3/20 5:15 PM, Thomas Huth wrote:
> On 03/07/2020 16.56, Peter Maydell wrote:
>> From: David CARLIER <devnexen@gmail.com>
>>
>> Regularize our handling of <sys/signal.h>: currently we include it in
>> osdep.h, but only for OpenBSD, and we include it without an ifdef
>> guard in a couple of C files.  This causes problems for Haiku, which
>> doesn't have that header.
>>
>> Instead, check in configure whether sys/signal.h exists, and if it
>> does then always include it from osdep.h.
>>
>> Signed-off-by: David Carlier <devnexen@gmail.com>
>> [PMM: Expanded commit message]
>> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>>  configure                   | 8 ++++++++
>>  include/qemu/osdep.h        | 2 +-
>>  hw/xen/xen-legacy-backend.c | 1 -
>>  util/oslib-posix.c          | 1 -
>>  4 files changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/configure b/configure
>> index ddc53d873ef..d131f760d8f 100755
>> --- a/configure
>> +++ b/configure
>> @@ -3212,6 +3212,11 @@ if ! check_include "ifaddrs.h" ; then
>>    have_ifaddrs_h=no
>>  fi
>>  
>> +have_sys_signal_h=no
>> +if check_include "sys/signal.h" ; then
>> +  have_sys_signal_h=yes
>> +fi
>> +
>>  ##########################################
>>  # VTE probe
>>  
>> @@ -7398,6 +7403,9 @@ fi
>>  if test "$have_broken_size_max" = "yes" ; then
>>      echo "HAVE_BROKEN_SIZE_MAX=y" >> $config_host_mak
>>  fi
>> +if test "$have_sys_signal_h" = "yes" ; then
>> +    echo "CONFIG_SYS_SIGNAL=y" >> $config_host_mak
>> +fi
> 
> I'd maybe rather name it HAVE_SYS_SIGNAL_H, but I guess that's just a
> matter of taste.

Agreed, if possible.
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> 
>