package/xen: Use POSIX complaint header includes

Submitted by Alistair Francis on April 17, 2017, 11:14 p.m.

Details

Message ID 20170417231457.2691-1-alistair.francis@xilinx.com
State New
Headers show

Commit Message

Alistair Francis April 17, 2017, 11:14 p.m.
To fix build issues when using the musl library use POSIX compatible
library inclues.

Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
---
 ...ls-Use-POSIX-poll.h-instead-of-sys-poll.h.patch | 79 ++++++++++++++++++++++
 ...se-POSIX-signal.h-instead-of-sys-signal.h.patch | 37 ++++++++++
 2 files changed, 116 insertions(+)
 create mode 100644 package/xen/0005-tools-Use-POSIX-poll.h-instead-of-sys-poll.h.patch
 create mode 100644 package/xen/0006-tools-Use-POSIX-signal.h-instead-of-sys-signal.h.patch

Comments

Baruch Siach April 18, 2017, 3:35 a.m.
Hi Alistair,

On Mon, Apr 17, 2017 at 04:14:57PM -0700, Alistair Francis wrote:
> To fix build issues when using the musl library use POSIX compatible
> library inclues.

Is there any build failure or run time issue that these patches are fixing?

It would be nice to have build warnings dealt with upstream to have a clean 
build. But Buildroot doesn't carry patches for the sole purpose of fixing 
build time warnings.

When build warnings break the build because of -Werror, we either add 
-Wno-error, or patch out the -Werror to fix the build.

baruch
Arnout Vandecappelle April 18, 2017, 8:04 a.m.
On 18-04-17 05:35, Baruch Siach wrote:
> Hi Alistair,
> 
> On Mon, Apr 17, 2017 at 04:14:57PM -0700, Alistair Francis wrote:
>> To fix build issues when using the musl library use POSIX compatible
>> library inclues.
> 
> Is there any build failure or run time issue that these patches are fixing?
> 
> It would be nice to have build warnings dealt with upstream to have a clean 
> build. But Buildroot doesn't carry patches for the sole purpose of fixing 
> build time warnings.

 +1 to that.

> When build warnings break the build because of -Werror, we either add 
> -Wno-error, or patch out the -Werror to fix the build.

 But not to this. A fundamental solution with an upstreamable patch is better,
-Wno-error is rather a stopgap measure when avoiding warnings is too difficult.
Which is often the case, by the way, e.g. if upstream haven't tested their code
yet with GCC 7.


 So, if this patch fixes a build or runtime failure, please mention this in the
commit message; if not, please mark the patch as Rejected in patchwork (but do
send it upstream, of course!).

 Regardless of this: thanks for the contribution, Alistair!

 Regards,
 Arnout
Arnout Vandecappelle April 18, 2017, 8:09 a.m.
On 18-04-17 10:04, Arnout Vandecappelle wrote:
> 
> 
> On 18-04-17 05:35, Baruch Siach wrote:
>> Hi Alistair,
>>
>> On Mon, Apr 17, 2017 at 04:14:57PM -0700, Alistair Francis wrote:
>>> To fix build issues when using the musl library use POSIX compatible
>>> library inclues.
>>
>> Is there any build failure or run time issue that these patches are fixing?
>>
>> It would be nice to have build warnings dealt with upstream to have a clean 
>> build. But Buildroot doesn't carry patches for the sole purpose of fixing 
>> build time warnings.
> 
>  +1 to that.
> 
>> When build warnings break the build because of -Werror, we either add 
>> -Wno-error, or patch out the -Werror to fix the build.
> 
>  But not to this. A fundamental solution with an upstreamable patch is better,
> -Wno-error is rather a stopgap measure when avoiding warnings is too difficult.
> Which is often the case, by the way, e.g. if upstream haven't tested their code
> yet with GCC 7.
> 
> 
>  So, if this patch fixes a build or runtime failure, please mention this in the
> commit message; if not, please mark the patch as Rejected in patchwork (but do
> send it upstream, of course!).

 So I checked myself, it does have an autobuild failure. Please add this to the
commit log:

Fixes:
http://autobuild.buildroot.net/results/1aa/1aa1303f60372f51aa5a7eb18caac4a5b5c1d9d4/build-end.log

 And also, show that you have submitted is upstream:

Submitted upstream as
https://www.mail-archive.com/xen-devel@lists.xen.org/msg105232.html

 With that,

Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>


 Regards,
 Arnout

> 
>  Regardless of this: thanks for the contribution, Alistair!
> 
>  Regards,
>  Arnout
>
Alistair Francis April 18, 2017, 3:54 p.m.
On Tue, Apr 18, 2017 at 1:09 AM, Arnout Vandecappelle <arnout@mind.be> wrote:
>
>
> On 18-04-17 10:04, Arnout Vandecappelle wrote:
>>
>>
>> On 18-04-17 05:35, Baruch Siach wrote:
>>> Hi Alistair,
>>>
>>> On Mon, Apr 17, 2017 at 04:14:57PM -0700, Alistair Francis wrote:
>>>> To fix build issues when using the musl library use POSIX compatible
>>>> library inclues.
>>>
>>> Is there any build failure or run time issue that these patches are fixing?
>>>
>>> It would be nice to have build warnings dealt with upstream to have a clean
>>> build. But Buildroot doesn't carry patches for the sole purpose of fixing
>>> build time warnings.
>>
>>  +1 to that.
>>
>>> When build warnings break the build because of -Werror, we either add
>>> -Wno-error, or patch out the -Werror to fix the build.
>>
>>  But not to this. A fundamental solution with an upstreamable patch is better,
>> -Wno-error is rather a stopgap measure when avoiding warnings is too difficult.
>> Which is often the case, by the way, e.g. if upstream haven't tested their code
>> yet with GCC 7.
>>
>>
>>  So, if this patch fixes a build or runtime failure, please mention this in the
>> commit message; if not, please mark the patch as Rejected in patchwork (but do
>> send it upstream, of course!).
>
>  So I checked myself, it does have an autobuild failure. Please add this to the
> commit log:
>
> Fixes:
> http://autobuild.buildroot.net/results/1aa/1aa1303f60372f51aa5a7eb18caac4a5b5c1d9d4/build-end.log

Thanks for the review.

Sorry I should have mentioned that this fixes a autobuild issue which
you already found.

>
>  And also, show that you have submitted is upstream:
>
> Submitted upstream as
> https://www.mail-archive.com/xen-devel@lists.xen.org/msg105232.html

I did submit it upstream and it looks like it will be accepted into
4.9 which means we can remove this in the future.

>
>  With that,
>
> Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

Thanks!

Alistair

>
>
>  Regards,
>  Arnout
>
>>
>>  Regardless of this: thanks for the contribution, Alistair!
>>
>>  Regards,
>>  Arnout
>>
>
> --
> Arnout Vandecappelle                          arnout at mind be
> Senior Embedded Software Architect            +32-16-286500
> Essensium/Mind                                http://www.mind.be
> G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
> LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
> GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

Patch hide | download patch | download mbox

diff --git a/package/xen/0005-tools-Use-POSIX-poll.h-instead-of-sys-poll.h.patch b/package/xen/0005-tools-Use-POSIX-poll.h-instead-of-sys-poll.h.patch
new file mode 100644
index 000000000..019c9b96e
--- /dev/null
+++ b/package/xen/0005-tools-Use-POSIX-poll.h-instead-of-sys-poll.h.patch
@@ -0,0 +1,79 @@ 
+From e407387926de4c75abd17bd1396caa95d35a4bea Mon Sep 17 00:00:00 2001
+From: Alistair Francis <alistair.francis@xilinx.com>
+Date: Mon, 17 Apr 2017 13:04:11 -0700
+Subject: [PATCH] tools: Use POSIX poll.h instead of sys/poll.h
+
+The POSIX spec specifies to use:
+    #include <poll.h>
+instead of:
+    #include <sys/poll.h>
+as seen here:
+    http://pubs.opengroup.org/onlinepubs/009695399/functions/poll.html
+
+This removes the warning:
+    #warning redirecting incorrect #include <sys/poll.h> to <poll.h>
+when building with the musl C-library.
+
+Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
+---
+ tools/libxl/libxl_internal.h               | 2 +-
+ tools/tests/xen-access/xen-access.c        | 2 +-
+ tools/xenstat/libxenstat/src/xenstat_qmp.c | 2 +-
+ tools/xentrace/xentrace.c                  | 2 +-
+ 4 files changed, 4 insertions(+), 4 deletions(-)
+
+diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
+index be24b76dfa..5d082c5704 100644
+--- a/tools/libxl/libxl_internal.h
++++ b/tools/libxl/libxl_internal.h
+@@ -38,7 +38,7 @@
+ #include <ctype.h>
+ 
+ #include <sys/mman.h>
+-#include <sys/poll.h>
++#include <poll.h>
+ #include <sys/select.h>
+ #include <sys/stat.h>
+ #include <sys/time.h>
+diff --git a/tools/tests/xen-access/xen-access.c b/tools/tests/xen-access/xen-access.c
+index ff4d289b45..238011e010 100644
+--- a/tools/tests/xen-access/xen-access.c
++++ b/tools/tests/xen-access/xen-access.c
+@@ -36,7 +36,7 @@
+ #include <signal.h>
+ #include <unistd.h>
+ #include <sys/mman.h>
+-#include <sys/poll.h>
++#include <poll.h>
+ 
+ #include <xenctrl.h>
+ #include <xenevtchn.h>
+diff --git a/tools/xenstat/libxenstat/src/xenstat_qmp.c b/tools/xenstat/libxenstat/src/xenstat_qmp.c
+index a87c9373c2..3fda487d49 100644
+--- a/tools/xenstat/libxenstat/src/xenstat_qmp.c
++++ b/tools/xenstat/libxenstat/src/xenstat_qmp.c
+@@ -14,7 +14,7 @@
+ #include <fcntl.h>
+ #include <sys/types.h>
+ #include <sys/socket.h>
+-#include <sys/poll.h>
++#include <poll.h>
+ #include <sys/un.h>
+ #include <stdlib.h>
+ #include <string.h>
+diff --git a/tools/xentrace/xentrace.c b/tools/xentrace/xentrace.c
+index f09fe6cf19..364a6fdad5 100644
+--- a/tools/xentrace/xentrace.c
++++ b/tools/xentrace/xentrace.c
+@@ -24,7 +24,7 @@
+ #include <getopt.h>
+ #include <assert.h>
+ #include <ctype.h>
+-#include <sys/poll.h>
++#include <poll.h>
+ #include <sys/statvfs.h>
+ 
+ #include <xen/xen.h>
+-- 
+2.11.0
+
diff --git a/package/xen/0006-tools-Use-POSIX-signal.h-instead-of-sys-signal.h.patch b/package/xen/0006-tools-Use-POSIX-signal.h-instead-of-sys-signal.h.patch
new file mode 100644
index 000000000..841feeb05
--- /dev/null
+++ b/package/xen/0006-tools-Use-POSIX-signal.h-instead-of-sys-signal.h.patch
@@ -0,0 +1,37 @@ 
+From 67315f02798cdccb186bd12dc5be94a7aec90852 Mon Sep 17 00:00:00 2001
+From: Alistair Francis <alistair.francis@xilinx.com>
+Date: Mon, 17 Apr 2017 14:15:54 -0700
+Subject: [PATCH 2/2] tools: Use POSIX signal.h instead of sys/signal.h
+
+The POSIX spec specifies to use:
+    #include <signal.h>
+instead of:
+    #include <sys/signal.h>
+as seen here:
+   http://pubs.opengroup.org/onlinepubs/009695399/functions/signal.html
+
+This removes the warning:
+    #warning redirecting incorrect #include <sys/signal.h> to <signal.h>
+when building with the musl C-library.
+
+Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
+---
+ tools/blktap2/drivers/tapdisk-server.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/tools/blktap2/drivers/tapdisk-server.c b/tools/blktap2/drivers/tapdisk-server.c
+index eecde3d23f..71315bb069 100644
+--- a/tools/blktap2/drivers/tapdisk-server.c
++++ b/tools/blktap2/drivers/tapdisk-server.c
+@@ -30,7 +30,7 @@
+ #include <unistd.h>
+ #include <stdlib.h>
+ #include <sys/ioctl.h>
+-#include <sys/signal.h>
++#include <signal.h>
+ 
+ #include "tapdisk-utils.h"
+ #include "tapdisk-server.h"
+-- 
+2.11.0
+