| Message ID | 602128d22db86bd67e11dec8fe40a73832c222c9.1559230347.git.baruch@tkos.co.il |
|---|---|
| State | Changes Requested |
| Delegated to: | stephen hemminger |
| Headers | show |
| Series | devlink: fix libc and kernel headers collision | expand |
Thu, May 30, 2019 at 05:32:27PM CEST, baruch@tkos.co.il wrote: >Since commit 2f1242efe9d ("devlink: Add devlink health show command") we >use the sys/sysinfo.h header for the sysinfo(2) system call. But since >iproute2 carries a local version of the kernel struct sysinfo, this >causes a collision with libc that do not rely on kernel defined sysinfo >like musl libc: > >In file included from devlink.c:25:0: >.../sysroot/usr/include/sys/sysinfo.h:10:8: error: redefinition of 'struct sysinfo' > struct sysinfo { > ^~~~~~~ >In file included from ../include/uapi/linux/kernel.h:5:0, > from ../include/uapi/linux/netlink.h:5, > from ../include/uapi/linux/genetlink.h:6, > from devlink.c:21: >../include/uapi/linux/sysinfo.h:8:8: note: originally defined here > struct sysinfo { > ^~~~~~~ > >Rely on the kernel header alone to avoid kernel and userspace headers >collision of definitions. > >Cc: Aya Levin <ayal@mellanox.com> >Cc: Moshe Shemesh <moshe@mellanox.com> >Signed-off-by: Baruch Siach <baruch@tkos.co.il> Acked-by: Jiri Pirko <jiri@mellanox.com>
Hi, > Since commit 2f1242efe9d ("devlink: Add devlink health show command") we > use the sys/sysinfo.h header for the sysinfo(2) system call. But since > iproute2 carries a local version of the kernel struct sysinfo, this > causes a collision with libc that do not rely on kernel defined sysinfo > like musl libc: > In file included from devlink.c:25:0: > .../sysroot/usr/include/sys/sysinfo.h:10:8: error: redefinition of 'struct sysinfo' > struct sysinfo { > ^~~~~~~ > In file included from ../include/uapi/linux/kernel.h:5:0, > from ../include/uapi/linux/netlink.h:5, > from ../include/uapi/linux/genetlink.h:6, > from devlink.c:21: > ../include/uapi/linux/sysinfo.h:8:8: note: originally defined here > struct sysinfo { > ^~~~~~~ > Rely on the kernel header alone to avoid kernel and userspace headers > collision of definitions. Reviewed-by: Petr Vorel <pvorel@suse.cz> Kind regards, Petr
On Thu, 30 May 2019 18:32:27 +0300 Baruch Siach <baruch@tkos.co.il> wrote: > Since commit 2f1242efe9d ("devlink: Add devlink health show command") we > use the sys/sysinfo.h header for the sysinfo(2) system call. But since > iproute2 carries a local version of the kernel struct sysinfo, this > causes a collision with libc that do not rely on kernel defined sysinfo > like musl libc: > > In file included from devlink.c:25:0: > .../sysroot/usr/include/sys/sysinfo.h:10:8: error: redefinition of 'struct sysinfo' > struct sysinfo { > ^~~~~~~ > In file included from ../include/uapi/linux/kernel.h:5:0, > from ../include/uapi/linux/netlink.h:5, > from ../include/uapi/linux/genetlink.h:6, > from devlink.c:21: > ../include/uapi/linux/sysinfo.h:8:8: note: originally defined here > struct sysinfo { > ^~~~~~~ > > Rely on the kernel header alone to avoid kernel and userspace headers > collision of definitions. > > Cc: Aya Levin <ayal@mellanox.com> > Cc: Moshe Shemesh <moshe@mellanox.com> > Signed-off-by: Baruch Siach <baruch@tkos.co.il> Ok, applied. Note that musl libc is not officially supported or tested as part of iproute2. I will take patches to fix build and bugs, but you are on your own if you must use it.
On Thu, 30 May 2019 18:32:27 +0300 Baruch Siach <baruch@tkos.co.il> wrote: > Since commit 2f1242efe9d ("devlink: Add devlink health show command") we > use the sys/sysinfo.h header for the sysinfo(2) system call. But since > iproute2 carries a local version of the kernel struct sysinfo, this > causes a collision with libc that do not rely on kernel defined sysinfo > like musl libc: > > In file included from devlink.c:25:0: > .../sysroot/usr/include/sys/sysinfo.h:10:8: error: redefinition of 'struct sysinfo' > struct sysinfo { > ^~~~~~~ > In file included from ../include/uapi/linux/kernel.h:5:0, > from ../include/uapi/linux/netlink.h:5, > from ../include/uapi/linux/genetlink.h:6, > from devlink.c:21: > ../include/uapi/linux/sysinfo.h:8:8: note: originally defined here > struct sysinfo { > ^~~~~~~ > > Rely on the kernel header alone to avoid kernel and userspace headers > collision of definitions. > > Cc: Aya Levin <ayal@mellanox.com> > Cc: Moshe Shemesh <moshe@mellanox.com> > Signed-off-by: Baruch Siach <baruch@tkos.co.il> Sorry this breaks the glibc build. CC devlink.o devlink.c: In function ‘format_logtime’: devlink.c:6124:8: warning: implicit declaration of function ‘sysinfo’; did you mean ‘psiginfo’? [-Wimplicit-function-declaration] err = sysinfo(&s_info); ^~~~~~~ psiginfo I backed out the patch now (before pushing it). Please fix and resubmit.
Hi Stephen, On Tue, Jun 04 2019, Stephen Hemminger wrote: > On Thu, 30 May 2019 18:32:27 +0300 > Baruch Siach <baruch@tkos.co.il> wrote: > >> Since commit 2f1242efe9d ("devlink: Add devlink health show command") we >> use the sys/sysinfo.h header for the sysinfo(2) system call. But since >> iproute2 carries a local version of the kernel struct sysinfo, this >> causes a collision with libc that do not rely on kernel defined sysinfo >> like musl libc: >> >> In file included from devlink.c:25:0: >> .../sysroot/usr/include/sys/sysinfo.h:10:8: error: redefinition of 'struct sysinfo' >> struct sysinfo { >> ^~~~~~~ >> In file included from ../include/uapi/linux/kernel.h:5:0, >> from ../include/uapi/linux/netlink.h:5, >> from ../include/uapi/linux/genetlink.h:6, >> from devlink.c:21: >> ../include/uapi/linux/sysinfo.h:8:8: note: originally defined here >> struct sysinfo { >> ^~~~~~~ >> >> Rely on the kernel header alone to avoid kernel and userspace headers >> collision of definitions. >> >> Cc: Aya Levin <ayal@mellanox.com> >> Cc: Moshe Shemesh <moshe@mellanox.com> >> Signed-off-by: Baruch Siach <baruch@tkos.co.il> > > Sorry this breaks the glibc build. > > > CC devlink.o > devlink.c: In function ‘format_logtime’: > devlink.c:6124:8: warning: implicit declaration of function ‘sysinfo’; did you mean ‘psiginfo’? [-Wimplicit-function-declaration] > err = sysinfo(&s_info); > ^~~~~~~ > psiginfo > > I backed out the patch now (before pushing it). > Please fix and resubmit. I can't think of anything better than this ugly fix: diff --git a/devlink/devlink.c b/devlink/devlink.c index 436935f88bda..02e648ef64b3 100644 --- a/devlink/devlink.c +++ b/devlink/devlink.c @@ -18,11 +18,12 @@ #include <limits.h> #include <errno.h> #include <inttypes.h> +#include <sys/sysinfo.h> +#define _LINUX_SYSINFO_H #include <linux/genetlink.h> #include <linux/devlink.h> #include <libmnl/libmnl.h> #include <netinet/ether.h> -#include <sys/sysinfo.h> #include <sys/queue.h> #include "SNAPSHOT.h" Would that be acceptable? baruch
diff --git a/devlink/devlink.c b/devlink/devlink.c index 436935f88bda..d7a6ce94f0e6 100644 --- a/devlink/devlink.c +++ b/devlink/devlink.c @@ -22,7 +22,7 @@ #include <linux/devlink.h> #include <libmnl/libmnl.h> #include <netinet/ether.h> -#include <sys/sysinfo.h> +#include <linux/sysinfo.h> #include <sys/queue.h> #include "SNAPSHOT.h"
Since commit 2f1242efe9d ("devlink: Add devlink health show command") we use the sys/sysinfo.h header for the sysinfo(2) system call. But since iproute2 carries a local version of the kernel struct sysinfo, this causes a collision with libc that do not rely on kernel defined sysinfo like musl libc: In file included from devlink.c:25:0: .../sysroot/usr/include/sys/sysinfo.h:10:8: error: redefinition of 'struct sysinfo' struct sysinfo { ^~~~~~~ In file included from ../include/uapi/linux/kernel.h:5:0, from ../include/uapi/linux/netlink.h:5, from ../include/uapi/linux/genetlink.h:6, from devlink.c:21: ../include/uapi/linux/sysinfo.h:8:8: note: originally defined here struct sysinfo { ^~~~~~~ Rely on the kernel header alone to avoid kernel and userspace headers collision of definitions. Cc: Aya Levin <ayal@mellanox.com> Cc: Moshe Shemesh <moshe@mellanox.com> Signed-off-by: Baruch Siach <baruch@tkos.co.il> --- devlink/devlink.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)