2020-09-30 22:41:28

by Petr Vorel

[permalink] [raw]
Subject: [PATCH 1/1] linux/sysinfo.h: Add guarder for struct sysinfo

for all but glibc libc.

This fixes redefinition on MUSL which also defines struct sysinfo when
including <linux/netlink.h> (which includes <linux/sysinfo.h> via
<linux/kernel.h>) and <sys/sysinfo.h>.

glibc loads <linux/sysinfo.h> in <sys/sysinfo.h>.

Signed-off-by: Petr Vorel <[email protected]>
---
include/uapi/linux/sysinfo.h | 3 +++
1 file changed, 3 insertions(+)

diff --git a/include/uapi/linux/sysinfo.h b/include/uapi/linux/sysinfo.h
index 435d5c23f0c0..c8ab18cd36b2 100644
--- a/include/uapi/linux/sysinfo.h
+++ b/include/uapi/linux/sysinfo.h
@@ -5,6 +5,8 @@
#include <linux/types.h>

#define SI_LOAD_SHIFT 16
+
+#if defined(__KERNEL__) || defined(__GLIBC__)
struct sysinfo {
__kernel_long_t uptime; /* Seconds since boot */
__kernel_ulong_t loads[3]; /* 1, 5, and 15 minute load averages */
@@ -21,5 +23,6 @@ struct sysinfo {
__u32 mem_unit; /* Memory unit size in bytes */
char _f[20-2*sizeof(__kernel_ulong_t)-sizeof(__u32)]; /* Padding: libc5 uses this.. */
};
+#endif

#endif /* _LINUX_SYSINFO_H */
--
2.27.0.rc0


2020-10-01 01:09:43

by Rich Felker

[permalink] [raw]
Subject: Re: [musl] [PATCH 1/1] linux/sysinfo.h: Add guarder for struct sysinfo

On Wed, Sep 30, 2020 at 11:46:36PM +0200, Petr Vorel wrote:
> for all but glibc libc.
>
> This fixes redefinition on MUSL which also defines struct sysinfo when
> including <linux/netlink.h> (which includes <linux/sysinfo.h> via
> <linux/kernel.h>) and <sys/sysinfo.h>.
>
> glibc loads <linux/sysinfo.h> in <sys/sysinfo.h>.
>
> Signed-off-by: Petr Vorel <[email protected]>
> ---
> include/uapi/linux/sysinfo.h | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/include/uapi/linux/sysinfo.h b/include/uapi/linux/sysinfo.h
> index 435d5c23f0c0..c8ab18cd36b2 100644
> --- a/include/uapi/linux/sysinfo.h
> +++ b/include/uapi/linux/sysinfo.h
> @@ -5,6 +5,8 @@
> #include <linux/types.h>
>
> #define SI_LOAD_SHIFT 16
> +
> +#if defined(__KERNEL__) || defined(__GLIBC__)
> struct sysinfo {
> __kernel_long_t uptime; /* Seconds since boot */
> __kernel_ulong_t loads[3]; /* 1, 5, and 15 minute load averages */
> @@ -21,5 +23,6 @@ struct sysinfo {
> __u32 mem_unit; /* Memory unit size in bytes */
> char _f[20-2*sizeof(__kernel_ulong_t)-sizeof(__u32)]; /* Padding: libc5 uses this.. */
> };
> +#endif
>
> #endif /* _LINUX_SYSINFO_H */
> --
> 2.27.0.rc0

I don't think this is the right way to do it. It prevents getting
access to the kernel uapi structure (which may be wanted) if you're
not using glibc or if you include kernel headers before any libc
headers. Rather, <linux/kernel.h>, whose only real purpose is
providing this structure to legacy applications that don't know the
renamed name for it, should not be implicitly included by other
headers. There's an existing thread on the topic but I don't have the
link handy. IIRC I proposed moving the alignment macros or whatever
other useful stuff is in <linux/kernel.h> to a separate header and
getting rid of all the indirect inclusions of <linux/kernel.h>.

Rich

2020-10-01 05:49:26

by Petr Vorel

[permalink] [raw]
Subject: Re: [musl] [PATCH 1/1] linux/sysinfo.h: Add guarder for struct sysinfo

Hi Rich,

...
> I don't think this is the right way to do it. It prevents getting
> access to the kernel uapi structure (which may be wanted) if you're
> not using glibc or if you include kernel headers before any libc
> headers. Rather, <linux/kernel.h>, whose only real purpose is
> providing this structure to legacy applications that don't know the
> renamed name for it, should not be implicitly included by other
> headers. There's an existing thread on the topic but I don't have the
> link handy. IIRC I proposed moving the alignment macros or whatever
> other useful stuff is in <linux/kernel.h> to a separate header and
> getting rid of all the indirect inclusions of <linux/kernel.h>.

Thanks for a review and tip!

Kind regards,
Petr