* [PATCH] Use __unused0 instead of __unused for user visible struct member names
@ 2012-01-02 2:44 Guillem Jover
2012-01-02 20:22 ` Jonathan Nieder
0 siblings, 1 reply; 11+ messages in thread
From: Guillem Jover @ 2012-01-02 2:44 UTC (permalink / raw)
To: linux-kernel
On BSD systems __unused has traditionally been defined to mean the
equivalent of gcc's __attribute__((__unused__)), some parts of the
Linux tree use that convention too (e.g. perf). The problem comes when
defining such macro while trying to build unmodified source code with
BSD origins on systems with Linux headers.
Rename the user visible struct members from __unused to __unused0 to
not cause compilation failures due to that macro, which should not be
a problem as those members are supposed to be private anyway.
Signed-off-by: Guillem Jover <guillem@hadrons.org>
---
arch/alpha/include/asm/stat.h | 2 +-
arch/arm/include/asm/ucontext.h | 2 +-
arch/ia64/include/asm/stat.h | 2 +-
arch/mips/include/asm/compat.h | 2 +-
arch/powerpc/include/asm/ucontext.h | 2 +-
arch/s390/include/asm/stat.h | 2 +-
arch/sparc/include/asm/compat.h | 4 ++--
arch/sparc/include/asm/elf_32.h | 2 +-
arch/sparc/include/asm/elf_64.h | 2 +-
arch/sparc/include/asm/fcntl.h | 4 ++--
arch/sparc/include/asm/stat.h | 2 +-
arch/tile/include/asm/compat.h | 2 +-
arch/x86/include/asm/stat.h | 2 +-
include/linux/icmp.h | 2 +-
include/linux/sysctl.h | 2 +-
15 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/arch/alpha/include/asm/stat.h b/arch/alpha/include/asm/stat.h
index 07ad3e6..5754865 100644
--- a/arch/alpha/include/asm/stat.h
+++ b/arch/alpha/include/asm/stat.h
@@ -42,7 +42,7 @@ struct stat64 {
unsigned long st_mtime_nsec;
unsigned long st_ctime;
unsigned long st_ctime_nsec;
- long __unused[3];
+ long __unused0[3];
};
#endif
diff --git a/arch/arm/include/asm/ucontext.h b/arch/arm/include/asm/ucontext.h
index 14749ae..7ea9743 100644
--- a/arch/arm/include/asm/ucontext.h
+++ b/arch/arm/include/asm/ucontext.h
@@ -20,7 +20,7 @@ struct ucontext {
struct sigcontext uc_mcontext;
sigset_t uc_sigmask;
/* Allow for uc_sigmask growth. Glibc uses a 1024-bit sigset_t. */
- int __unused[32 - (sizeof (sigset_t) / sizeof (int))];
+ int __unused0[32 - (sizeof (sigset_t) / sizeof (int))];
/* Last for extensibility. Eight byte aligned because some
coprocessors require eight byte alignment. */
unsigned long uc_regspace[128] __attribute__((__aligned__(8)));
diff --git a/arch/ia64/include/asm/stat.h b/arch/ia64/include/asm/stat.h
index 367bb90..2ec0d4c 100644
--- a/arch/ia64/include/asm/stat.h
+++ b/arch/ia64/include/asm/stat.h
@@ -24,7 +24,7 @@ struct stat {
unsigned long st_ctime_nsec;
unsigned long st_blksize;
long st_blocks;
- unsigned long __unused[3];
+ unsigned long __unused0[3];
};
#define STAT_HAVE_NSEC 1
diff --git a/arch/mips/include/asm/compat.h b/arch/mips/include/asm/compat.h
index b77df03..333d49f 100644
--- a/arch/mips/include/asm/compat.h
+++ b/arch/mips/include/asm/compat.h
@@ -84,7 +84,7 @@ struct compat_flock {
compat_off_t l_len;
s32 l_sysid;
compat_pid_t l_pid;
- short __unused;
+ short __unused0;
s32 pad[4];
};
diff --git a/arch/powerpc/include/asm/ucontext.h b/arch/powerpc/include/asm/ucontext.h
index d9a4ddf..6121f9e 100644
--- a/arch/powerpc/include/asm/ucontext.h
+++ b/arch/powerpc/include/asm/ucontext.h
@@ -28,7 +28,7 @@ struct ucontext {
sigset_t uc_sigmask;
/* glibc has 1024-bit signal masks, ours are 64-bit */
#ifdef __powerpc64__
- sigset_t __unused[15]; /* Allow for uc_sigmask growth */
+ sigset_t __unused0[15]; /* Allow for uc_sigmask growth */
struct sigcontext uc_mcontext; /* last for extensibility */
#else
int uc_maskext[30];
diff --git a/arch/s390/include/asm/stat.h b/arch/s390/include/asm/stat.h
index d92959e..9577947 100644
--- a/arch/s390/include/asm/stat.h
+++ b/arch/s390/include/asm/stat.h
@@ -95,7 +95,7 @@ struct stat {
unsigned long st_ctime_nsec;
unsigned long st_blksize;
long st_blocks;
- unsigned long __unused[3];
+ unsigned long __unused0[3];
};
#endif /* __s390x__ */
diff --git a/arch/sparc/include/asm/compat.h b/arch/sparc/include/asm/compat.h
index b8be20d..8f20999 100644
--- a/arch/sparc/include/asm/compat.h
+++ b/arch/sparc/include/asm/compat.h
@@ -107,7 +107,7 @@ struct compat_flock {
compat_off_t l_start;
compat_off_t l_len;
compat_pid_t l_pid;
- short __unused;
+ short __unused0;
};
#define F_GETLK64 12
@@ -120,7 +120,7 @@ struct compat_flock64 {
compat_loff_t l_start;
compat_loff_t l_len;
compat_pid_t l_pid;
- short __unused;
+ short __unused0;
};
struct compat_statfs {
diff --git a/arch/sparc/include/asm/elf_32.h b/arch/sparc/include/asm/elf_32.h
index 4269ca6..7dffbe2 100644
--- a/arch/sparc/include/asm/elf_32.h
+++ b/arch/sparc/include/asm/elf_32.h
@@ -82,7 +82,7 @@ typedef struct {
unsigned long pr_regs[32];
double pr_dregs[16];
} pr_fr;
- unsigned long __unused;
+ unsigned long __unused0;
unsigned long pr_fsr;
unsigned char pr_qcnt;
unsigned char pr_q_entrysize;
diff --git a/arch/sparc/include/asm/elf_64.h b/arch/sparc/include/asm/elf_64.h
index 7df8b7f..1db74f5 100644
--- a/arch/sparc/include/asm/elf_64.h
+++ b/arch/sparc/include/asm/elf_64.h
@@ -133,7 +133,7 @@ typedef struct {
unsigned int pr_regs[32];
unsigned long pr_dregs[16];
} pr_fr;
- unsigned int __unused;
+ unsigned int __unused0;
unsigned int pr_fsr;
unsigned char pr_qcnt;
unsigned char pr_q_entrysize;
diff --git a/arch/sparc/include/asm/fcntl.h b/arch/sparc/include/asm/fcntl.h
index d0b83f6..a5e8516 100644
--- a/arch/sparc/include/asm/fcntl.h
+++ b/arch/sparc/include/asm/fcntl.h
@@ -47,8 +47,8 @@
#define F_WRLCK 2
#define F_UNLCK 3
-#define __ARCH_FLOCK_PAD short __unused;
-#define __ARCH_FLOCK64_PAD short __unused;
+#define __ARCH_FLOCK_PAD short __unused0;
+#define __ARCH_FLOCK64_PAD short __unused0;
#include <asm-generic/fcntl.h>
diff --git a/arch/sparc/include/asm/stat.h b/arch/sparc/include/asm/stat.h
index a232e9e..7258fb4 100644
--- a/arch/sparc/include/asm/stat.h
+++ b/arch/sparc/include/asm/stat.h
@@ -43,7 +43,7 @@ struct stat64 {
unsigned long st_mtime_nsec;
unsigned long st_ctime;
unsigned long st_ctime_nsec;
- long __unused[3];
+ long __unused0[3];
};
#else
diff --git a/arch/tile/include/asm/compat.h b/arch/tile/include/asm/compat.h
index bf95f55..6f31f94 100644
--- a/arch/tile/include/asm/compat.h
+++ b/arch/tile/include/asm/compat.h
@@ -80,7 +80,7 @@ struct compat_sysctl {
unsigned int oldlenp;
unsigned int newval;
unsigned int newlen;
- unsigned int __unused[4];
+ unsigned int __unused0[4];
};
diff --git a/arch/x86/include/asm/stat.h b/arch/x86/include/asm/stat.h
index e0b1d9b..a7fb9e5 100644
--- a/arch/x86/include/asm/stat.h
+++ b/arch/x86/include/asm/stat.h
@@ -85,7 +85,7 @@ struct stat {
unsigned long st_mtime_nsec;
unsigned long st_ctime;
unsigned long st_ctime_nsec;
- long __unused[3];
+ long __unused0[3];
};
#endif
diff --git a/include/linux/icmp.h b/include/linux/icmp.h
index 474f2a5..124e861 100644
--- a/include/linux/icmp.h
+++ b/include/linux/icmp.h
@@ -76,7 +76,7 @@ struct icmphdr {
} echo;
__be32 gateway;
struct {
- __be16 __unused;
+ __be16 __unused0;
__be16 mtu;
} frag;
} un;
diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index 703cfa3..9911ac5 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -40,7 +40,7 @@ struct __sysctl_args {
size_t __user *oldlenp;
void __user *newval;
size_t newlen;
- unsigned long __unused[4];
+ unsigned long __unused0[4];
};
/* Define sysctl names first */
--
1.7.7.3
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] Use __unused0 instead of __unused for user visible struct member names
2012-01-02 2:44 [PATCH] Use __unused0 instead of __unused for user visible struct member names Guillem Jover
@ 2012-01-02 20:22 ` Jonathan Nieder
2012-01-03 6:56 ` Sam Ravnborg
0 siblings, 1 reply; 11+ messages in thread
From: Jonathan Nieder @ 2012-01-02 20:22 UTC (permalink / raw)
To: Guillem Jover
Cc: linux-kernel, linux-kbuild, linux-api, Sam Ravnborg, Michal Marek
(+cc: some possible relevant people)
Hi,
Guillem Jover wrote:
> On BSD systems __unused has traditionally been defined to mean the
> equivalent of gcc's __attribute__((__unused__)), some parts of the
> Linux tree use that convention too (e.g. perf). The problem comes when
> defining such macro while trying to build unmodified source code with
> BSD origins on systems with Linux headers.
>
> Rename the user visible struct members from __unused to __unused0 to
> not cause compilation failures due to that macro, which should not be
> a problem as those members are supposed to be private anyway.
Thanks, sounds like a good idea.
It would be nice to document one detail: why do these struct members
have a double-underscore before "unused" in the first place? I
understand that libc has (and hence installed kernel headers have) a
limited namespace to work with and they have to be on guard against
applications doing something crazy like
#define unused 0
but aren't there other identifiers in that namespace that work just as
well?
That's not actually an objection --- I'm just curious. __unused1 like
is already used in e.g. asm-generic/stat.h seems fine to me.
Some examples:
> Signed-off-by: Guillem Jover <guillem@hadrons.org>
> ---
[...]
> diff --git a/arch/alpha/include/asm/stat.h b/arch/alpha/include/asm/stat.h
> index 07ad3e6..5754865 100644
> --- a/arch/alpha/include/asm/stat.h
> +++ b/arch/alpha/include/asm/stat.h
> @@ -42,7 +42,7 @@ struct stat64 {
> unsigned long st_mtime_nsec;
> unsigned long st_ctime;
> unsigned long st_ctime_nsec;
> - long __unused[3];
> + long __unused0[3];
> };
This could be
long st_unused[3];
or
long st_padding[3];
> #endif
> diff --git a/arch/arm/include/asm/ucontext.h b/arch/arm/include/asm/ucontext.h
> index 14749ae..7ea9743 100644
> --- a/arch/arm/include/asm/ucontext.h
> +++ b/arch/arm/include/asm/ucontext.h
> @@ -20,7 +20,7 @@ struct ucontext {
> struct sigcontext uc_mcontext;
> sigset_t uc_sigmask;
> /* Allow for uc_sigmask growth. Glibc uses a 1024-bit sigset_t. */
> - int __unused[32 - (sizeof (sigset_t) / sizeof (int))];
> + int __unused0[32 - (sizeof (sigset_t) / sizeof (int))];
> /* Last for extensibility. Eight byte aligned because some
> coprocessors require eight byte alignment. */
> unsigned long uc_regspace[128] __attribute__((__aligned__(8)));
Likewise, this could be something like
int uc_sigmask_padding[32 - ...];
Remainder left unsnipped for convenience of people cc-ed. If the
general idea looks sensible, what would be the best way to usher it
into mainline? Should it be split up into patches for the various
arch-specific trees, or should acks from relevant people be collected
some other way?
Thanks,
Jonathan
> diff --git a/arch/ia64/include/asm/stat.h b/arch/ia64/include/asm/stat.h
> index 367bb90..2ec0d4c 100644
> --- a/arch/ia64/include/asm/stat.h
> +++ b/arch/ia64/include/asm/stat.h
> @@ -24,7 +24,7 @@ struct stat {
> unsigned long st_ctime_nsec;
> unsigned long st_blksize;
> long st_blocks;
> - unsigned long __unused[3];
> + unsigned long __unused0[3];
> };
>
> #define STAT_HAVE_NSEC 1
> diff --git a/arch/mips/include/asm/compat.h b/arch/mips/include/asm/compat.h
> index b77df03..333d49f 100644
> --- a/arch/mips/include/asm/compat.h
> +++ b/arch/mips/include/asm/compat.h
> @@ -84,7 +84,7 @@ struct compat_flock {
> compat_off_t l_len;
> s32 l_sysid;
> compat_pid_t l_pid;
> - short __unused;
> + short __unused0;
> s32 pad[4];
> };
>
> diff --git a/arch/powerpc/include/asm/ucontext.h b/arch/powerpc/include/asm/ucontext.h
> index d9a4ddf..6121f9e 100644
> --- a/arch/powerpc/include/asm/ucontext.h
> +++ b/arch/powerpc/include/asm/ucontext.h
> @@ -28,7 +28,7 @@ struct ucontext {
> sigset_t uc_sigmask;
> /* glibc has 1024-bit signal masks, ours are 64-bit */
> #ifdef __powerpc64__
> - sigset_t __unused[15]; /* Allow for uc_sigmask growth */
> + sigset_t __unused0[15]; /* Allow for uc_sigmask growth */
> struct sigcontext uc_mcontext; /* last for extensibility */
> #else
> int uc_maskext[30];
> diff --git a/arch/s390/include/asm/stat.h b/arch/s390/include/asm/stat.h
> index d92959e..9577947 100644
> --- a/arch/s390/include/asm/stat.h
> +++ b/arch/s390/include/asm/stat.h
> @@ -95,7 +95,7 @@ struct stat {
> unsigned long st_ctime_nsec;
> unsigned long st_blksize;
> long st_blocks;
> - unsigned long __unused[3];
> + unsigned long __unused0[3];
> };
>
> #endif /* __s390x__ */
> diff --git a/arch/sparc/include/asm/compat.h b/arch/sparc/include/asm/compat.h
> index b8be20d..8f20999 100644
> --- a/arch/sparc/include/asm/compat.h
> +++ b/arch/sparc/include/asm/compat.h
> @@ -107,7 +107,7 @@ struct compat_flock {
> compat_off_t l_start;
> compat_off_t l_len;
> compat_pid_t l_pid;
> - short __unused;
> + short __unused0;
> };
>
> #define F_GETLK64 12
> @@ -120,7 +120,7 @@ struct compat_flock64 {
> compat_loff_t l_start;
> compat_loff_t l_len;
> compat_pid_t l_pid;
> - short __unused;
> + short __unused0;
> };
>
> struct compat_statfs {
> diff --git a/arch/sparc/include/asm/elf_32.h b/arch/sparc/include/asm/elf_32.h
> index 4269ca6..7dffbe2 100644
> --- a/arch/sparc/include/asm/elf_32.h
> +++ b/arch/sparc/include/asm/elf_32.h
> @@ -82,7 +82,7 @@ typedef struct {
> unsigned long pr_regs[32];
> double pr_dregs[16];
> } pr_fr;
> - unsigned long __unused;
> + unsigned long __unused0;
> unsigned long pr_fsr;
> unsigned char pr_qcnt;
> unsigned char pr_q_entrysize;
> diff --git a/arch/sparc/include/asm/elf_64.h b/arch/sparc/include/asm/elf_64.h
> index 7df8b7f..1db74f5 100644
> --- a/arch/sparc/include/asm/elf_64.h
> +++ b/arch/sparc/include/asm/elf_64.h
> @@ -133,7 +133,7 @@ typedef struct {
> unsigned int pr_regs[32];
> unsigned long pr_dregs[16];
> } pr_fr;
> - unsigned int __unused;
> + unsigned int __unused0;
> unsigned int pr_fsr;
> unsigned char pr_qcnt;
> unsigned char pr_q_entrysize;
> diff --git a/arch/sparc/include/asm/fcntl.h b/arch/sparc/include/asm/fcntl.h
> index d0b83f6..a5e8516 100644
> --- a/arch/sparc/include/asm/fcntl.h
> +++ b/arch/sparc/include/asm/fcntl.h
> @@ -47,8 +47,8 @@
> #define F_WRLCK 2
> #define F_UNLCK 3
>
> -#define __ARCH_FLOCK_PAD short __unused;
> -#define __ARCH_FLOCK64_PAD short __unused;
> +#define __ARCH_FLOCK_PAD short __unused0;
> +#define __ARCH_FLOCK64_PAD short __unused0;
>
> #include <asm-generic/fcntl.h>
>
> diff --git a/arch/sparc/include/asm/stat.h b/arch/sparc/include/asm/stat.h
> index a232e9e..7258fb4 100644
> --- a/arch/sparc/include/asm/stat.h
> +++ b/arch/sparc/include/asm/stat.h
> @@ -43,7 +43,7 @@ struct stat64 {
> unsigned long st_mtime_nsec;
> unsigned long st_ctime;
> unsigned long st_ctime_nsec;
> - long __unused[3];
> + long __unused0[3];
> };
>
> #else
> diff --git a/arch/tile/include/asm/compat.h b/arch/tile/include/asm/compat.h
> index bf95f55..6f31f94 100644
> --- a/arch/tile/include/asm/compat.h
> +++ b/arch/tile/include/asm/compat.h
> @@ -80,7 +80,7 @@ struct compat_sysctl {
> unsigned int oldlenp;
> unsigned int newval;
> unsigned int newlen;
> - unsigned int __unused[4];
> + unsigned int __unused0[4];
> };
>
>
> diff --git a/arch/x86/include/asm/stat.h b/arch/x86/include/asm/stat.h
> index e0b1d9b..a7fb9e5 100644
> --- a/arch/x86/include/asm/stat.h
> +++ b/arch/x86/include/asm/stat.h
> @@ -85,7 +85,7 @@ struct stat {
> unsigned long st_mtime_nsec;
> unsigned long st_ctime;
> unsigned long st_ctime_nsec;
> - long __unused[3];
> + long __unused0[3];
> };
> #endif
>
> diff --git a/include/linux/icmp.h b/include/linux/icmp.h
> index 474f2a5..124e861 100644
> --- a/include/linux/icmp.h
> +++ b/include/linux/icmp.h
> @@ -76,7 +76,7 @@ struct icmphdr {
> } echo;
> __be32 gateway;
> struct {
> - __be16 __unused;
> + __be16 __unused0;
> __be16 mtu;
> } frag;
> } un;
> diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
> index 703cfa3..9911ac5 100644
> --- a/include/linux/sysctl.h
> +++ b/include/linux/sysctl.h
> @@ -40,7 +40,7 @@ struct __sysctl_args {
> size_t __user *oldlenp;
> void __user *newval;
> size_t newlen;
> - unsigned long __unused[4];
> + unsigned long __unused0[4];
> };
>
> /* Define sysctl names first */
> --
> 1.7.7.3
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Use __unused0 instead of __unused for user visible struct member names
@ 2012-01-03 6:56 ` Sam Ravnborg
0 siblings, 0 replies; 11+ messages in thread
From: Sam Ravnborg @ 2012-01-03 6:56 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Guillem Jover, linux-kernel, linux-kbuild, linux-api, Michal Marek
On Mon, Jan 02, 2012 at 02:22:43PM -0600, Jonathan Nieder wrote:
> (+cc: some possible relevant people)
> Hi,
>
> Guillem Jover wrote:
>
> > On BSD systems __unused has traditionally been defined to mean the
> > equivalent of gcc's __attribute__((__unused__)), some parts of the
> > Linux tree use that convention too (e.g. perf). The problem comes when
> > defining such macro while trying to build unmodified source code with
> > BSD origins on systems with Linux headers.
> >
> > Rename the user visible struct members from __unused to __unused0 to
> > not cause compilation failures due to that macro, which should not be
> > a problem as those members are supposed to be private anyway.
^__ is reserved for libc internal stuff and there is no reason to
name the unused/padding members "__unused".
So one or a set of patches that rename them all to something more
sensible would be fine.
Sam
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Use __unused0 instead of __unused for user visible struct member names
@ 2012-01-03 6:56 ` Sam Ravnborg
0 siblings, 0 replies; 11+ messages in thread
From: Sam Ravnborg @ 2012-01-03 6:56 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Guillem Jover, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-kbuild-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, Michal Marek
On Mon, Jan 02, 2012 at 02:22:43PM -0600, Jonathan Nieder wrote:
> (+cc: some possible relevant people)
> Hi,
>
> Guillem Jover wrote:
>
> > On BSD systems __unused has traditionally been defined to mean the
> > equivalent of gcc's __attribute__((__unused__)), some parts of the
> > Linux tree use that convention too (e.g. perf). The problem comes when
> > defining such macro while trying to build unmodified source code with
> > BSD origins on systems with Linux headers.
> >
> > Rename the user visible struct members from __unused to __unused0 to
> > not cause compilation failures due to that macro, which should not be
> > a problem as those members are supposed to be private anyway.
^__ is reserved for libc internal stuff and there is no reason to
name the unused/padding members "__unused".
So one or a set of patches that rename them all to something more
sensible would be fine.
Sam
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Use __unused0 instead of __unused for user visible struct member names
2012-01-03 6:56 ` Sam Ravnborg
(?)
@ 2012-01-04 8:14 ` Guillem Jover
2012-01-04 11:03 ` Michal Marek
-1 siblings, 1 reply; 11+ messages in thread
From: Guillem Jover @ 2012-01-04 8:14 UTC (permalink / raw)
To: Sam Ravnborg
Cc: Jonathan Nieder, linux-kernel, linux-kbuild, linux-api, Michal Marek
On Tue, 2012-01-03 at 07:56:59 +0100, Sam Ravnborg wrote:
> On Mon, Jan 02, 2012 at 02:22:43PM -0600, Jonathan Nieder wrote:
> > Guillem Jover wrote:
> > > On BSD systems __unused has traditionally been defined to mean the
> > > equivalent of gcc's __attribute__((__unused__)), some parts of the
> > > Linux tree use that convention too (e.g. perf). The problem comes when
> > > defining such macro while trying to build unmodified source code with
> > > BSD origins on systems with Linux headers.
> > >
> > > Rename the user visible struct members from __unused to __unused0 to
> > > not cause compilation failures due to that macro, which should not be
> > > a problem as those members are supposed to be private anyway.
>
> ^__ is reserved for libc internal stuff and there is no reason to
> name the unused/padding members "__unused".
> So one or a set of patches that rename them all to something more
> sensible would be fine.
On a quick glance, I've found other functionally similar struct
member names present on the tree:
__unused __unusedN __reserved __reservedN __reserved_N __resN
__pad __padN __flr_pad __ifi_pad __tcpm_padN __tcpct_padN
Do you mean you'd like to see patch(es) to rename all those? I'd not
mind providing them, although my immediate concern right now is just
regarding __unused.
There's also __buf in linux/sem.h and __data in linux/socket.h, but
I'd rather not thouch those, as I'd expect to be users for them?
thanks,
guillem
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Use __unused0 instead of __unused for user visible struct member names
2012-01-04 8:14 ` Guillem Jover
@ 2012-01-04 11:03 ` Michal Marek
2012-01-04 13:35 ` Sam Ravnborg
0 siblings, 1 reply; 11+ messages in thread
From: Michal Marek @ 2012-01-04 11:03 UTC (permalink / raw)
To: Guillem Jover
Cc: Sam Ravnborg, Jonathan Nieder, linux-kernel, linux-kbuild, linux-api
On 4.1.2012 09:14, Guillem Jover wrote:
> On Tue, 2012-01-03 at 07:56:59 +0100, Sam Ravnborg wrote:
>> On Mon, Jan 02, 2012 at 02:22:43PM -0600, Jonathan Nieder wrote:
>>> Guillem Jover wrote:
>>>> On BSD systems __unused has traditionally been defined to mean the
>>>> equivalent of gcc's __attribute__((__unused__)), some parts of the
>>>> Linux tree use that convention too (e.g. perf). The problem comes when
>>>> defining such macro while trying to build unmodified source code with
>>>> BSD origins on systems with Linux headers.
>>>>
>>>> Rename the user visible struct members from __unused to __unused0 to
>>>> not cause compilation failures due to that macro, which should not be
>>>> a problem as those members are supposed to be private anyway.
>>
>> ^__ is reserved for libc internal stuff and there is no reason to
>> name the unused/padding members "__unused".
>> So one or a set of patches that rename them all to something more
>> sensible would be fine.
>
> On a quick glance, I've found other functionally similar struct
> member names present on the tree:
>
> __unused __unusedN __reserved __reservedN __reserved_N __resN
> __pad __padN __flr_pad __ifi_pad __tcpm_padN __tcpct_padN
>
> Do you mean you'd like to see patch(es) to rename all those? I'd not
> mind providing them, although my immediate concern right now is just
> regarding __unused.
__.* and _[A-Z].* are reserved for the implementation. Unfortunately,
both the kernel userspace headers and the libc are part of the
implementation, so there needs to be some common sense applied to avoid
clashes. IMO renaming __unused to __unused0 on the basis that some
headers define __unused to __attribute__((__unused__)) makes sense, but
blindly renaming any occurence of double underscore helps little.
Just my $0.02.
Michal
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Use __unused0 instead of __unused for user visible struct member names
@ 2012-01-04 13:35 ` Sam Ravnborg
0 siblings, 0 replies; 11+ messages in thread
From: Sam Ravnborg @ 2012-01-04 13:35 UTC (permalink / raw)
To: Michal Marek
Cc: Guillem Jover, Jonathan Nieder, linux-kernel, linux-kbuild, linux-api
On Wed, Jan 04, 2012 at 12:03:16PM +0100, Michal Marek wrote:
> On 4.1.2012 09:14, Guillem Jover wrote:
> > On Tue, 2012-01-03 at 07:56:59 +0100, Sam Ravnborg wrote:
> >> On Mon, Jan 02, 2012 at 02:22:43PM -0600, Jonathan Nieder wrote:
> >>> Guillem Jover wrote:
> >>>> On BSD systems __unused has traditionally been defined to mean the
> >>>> equivalent of gcc's __attribute__((__unused__)), some parts of the
> >>>> Linux tree use that convention too (e.g. perf). The problem comes when
> >>>> defining such macro while trying to build unmodified source code with
> >>>> BSD origins on systems with Linux headers.
> >>>>
> >>>> Rename the user visible struct members from __unused to __unused0 to
> >>>> not cause compilation failures due to that macro, which should not be
> >>>> a problem as those members are supposed to be private anyway.
> >>
> >> ^__ is reserved for libc internal stuff and there is no reason to
> >> name the unused/padding members "__unused".
> >> So one or a set of patches that rename them all to something more
> >> sensible would be fine.
> >
> > On a quick glance, I've found other functionally similar struct
> > member names present on the tree:
> >
> > __unused __unusedN __reserved __reservedN __reserved_N __resN
> > __pad __padN __flr_pad __ifi_pad __tcpm_padN __tcpct_padN
> >
> > Do you mean you'd like to see patch(es) to rename all those? I'd not
> > mind providing them, although my immediate concern right now is just
> > regarding __unused.
>
> __.* and _[A-Z].* are reserved for the implementation. Unfortunately,
> both the kernel userspace headers and the libc are part of the
> implementation, so there needs to be some common sense applied to avoid
> clashes. IMO renaming __unused to __unused0 on the basis that some
> headers define __unused to __attribute__((__unused__)) makes sense, but
> blindly renaming any occurence of double underscore helps little.
Agree on Michael on this.
Sam
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Use __unused0 instead of __unused for user visible struct member names
@ 2012-01-04 13:35 ` Sam Ravnborg
0 siblings, 0 replies; 11+ messages in thread
From: Sam Ravnborg @ 2012-01-04 13:35 UTC (permalink / raw)
To: Michal Marek
Cc: Guillem Jover, Jonathan Nieder,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-kbuild-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA
On Wed, Jan 04, 2012 at 12:03:16PM +0100, Michal Marek wrote:
> On 4.1.2012 09:14, Guillem Jover wrote:
> > On Tue, 2012-01-03 at 07:56:59 +0100, Sam Ravnborg wrote:
> >> On Mon, Jan 02, 2012 at 02:22:43PM -0600, Jonathan Nieder wrote:
> >>> Guillem Jover wrote:
> >>>> On BSD systems __unused has traditionally been defined to mean the
> >>>> equivalent of gcc's __attribute__((__unused__)), some parts of the
> >>>> Linux tree use that convention too (e.g. perf). The problem comes when
> >>>> defining such macro while trying to build unmodified source code with
> >>>> BSD origins on systems with Linux headers.
> >>>>
> >>>> Rename the user visible struct members from __unused to __unused0 to
> >>>> not cause compilation failures due to that macro, which should not be
> >>>> a problem as those members are supposed to be private anyway.
> >>
> >> ^__ is reserved for libc internal stuff and there is no reason to
> >> name the unused/padding members "__unused".
> >> So one or a set of patches that rename them all to something more
> >> sensible would be fine.
> >
> > On a quick glance, I've found other functionally similar struct
> > member names present on the tree:
> >
> > __unused __unusedN __reserved __reservedN __reserved_N __resN
> > __pad __padN __flr_pad __ifi_pad __tcpm_padN __tcpct_padN
> >
> > Do you mean you'd like to see patch(es) to rename all those? I'd not
> > mind providing them, although my immediate concern right now is just
> > regarding __unused.
>
> __.* and _[A-Z].* are reserved for the implementation. Unfortunately,
> both the kernel userspace headers and the libc are part of the
> implementation, so there needs to be some common sense applied to avoid
> clashes. IMO renaming __unused to __unused0 on the basis that some
> headers define __unused to __attribute__((__unused__)) makes sense, but
> blindly renaming any occurence of double underscore helps little.
Agree on Michael on this.
Sam
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Use __unused0 instead of __unused for user visible struct member names
2012-01-03 6:56 ` Sam Ravnborg
@ 2013-11-11 14:59 ` Thorsten Glaser
-1 siblings, 0 replies; 11+ messages in thread
From: Thorsten Glaser @ 2013-11-11 14:59 UTC (permalink / raw)
To: Sam Ravnborg
Cc: Jonathan Nieder, Guillem Jover,
public-linux-kernel-u79uwXL29TY76Z2rM5mHXA,
public-linux-kbuild-u79uwXL29TY76Z2rM5mHXA,
public-linux-api-u79uwXL29TY76Z2rM5mHXA, Michal Marek,
Justin Cormack
Sam Ravnborg dixit:
>> Guillem Jover wrote:
>> > On BSD systems __unused has traditionally been defined to mean the
>> > equivalent of gcc's __attribute__((__unused__)), some parts of the
[…]
>^__ is reserved for libc internal stuff and there is no reason to
>name the unused/padding members "__unused".
Considering that glibc has seen the light¹ now too, can we please
do something about these now? The BSD tools (not just NetBSD®) have
been using this for far longer after all…
Currently (git pull --ff torvalds master), we have:
• 2 occurrences of files *inside* the Linux kernel defining __unused
to² __attribute__((unused)) themselves
• 68 struct members and function arguments called __unused
>So one or a set of patches that rename them all to something more
>sensible would be fine.
I think __unused0 is okay as it matches current __unused[0-9] in
use by other parts of the Linux kernel – although glibc now uses
__glibc_reserved[0-9], I think this doesn’t look like the Linux
kernel should use it ☻☺
① http://thread.gmane.org/gmane.comp.lib.glibc.alpha/36439
② I’ve recently come to the belief that this should be
__attribute__((__unused__)) in all cases, i.e. all those
attribute namings need double underscores before and after,
as some software likes to #define printf to something else,
lighttpd does #define bounded something else, so there’s
probably software out there containing #define unused foo.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Use __unused0 instead of __unused for user visible struct member names
@ 2013-11-11 14:59 ` Thorsten Glaser
0 siblings, 0 replies; 11+ messages in thread
From: Thorsten Glaser @ 2013-11-11 14:59 UTC (permalink / raw)
To: Sam Ravnborg
Cc: Jonathan Nieder, Guillem Jover,
public-linux-kernel-u79uwXL29TY76Z2rM5mHXA,
public-linux-kbuild-u79uwXL29TY76Z2rM5mHXA,
public-linux-api-u79uwXL29TY76Z2rM5mHXA, Michal Marek,
Justin Cormack
Sam Ravnborg dixit:
>> Guillem Jover wrote:
>> > On BSD systems __unused has traditionally been defined to mean the
>> > equivalent of gcc's __attribute__((__unused__)), some parts of the
[…]
>^__ is reserved for libc internal stuff and there is no reason to
>name the unused/padding members "__unused".
Considering that glibc has seen the light¹ now too, can we please
do something about these now? The BSD tools (not just NetBSD®) have
been using this for far longer after all…
Currently (git pull --ff torvalds master), we have:
• 2 occurrences of files *inside* the Linux kernel defining __unused
to² __attribute__((unused)) themselves
• 68 struct members and function arguments called __unused
>So one or a set of patches that rename them all to something more
>sensible would be fine.
I think __unused0 is okay as it matches current __unused[0-9] in
use by other parts of the Linux kernel – although glibc now uses
__glibc_reserved[0-9], I think this doesn’t look like the Linux
kernel should use it ☻☺
① http://thread.gmane.org/gmane.comp.lib.glibc.alpha/36439
② I’ve recently come to the belief that this should be
__attribute__((__unused__)) in all cases, i.e. all those
attribute namings need double underscores before and after,
as some software likes to #define printf to something else,
lighttpd does #define bounded something else, so there’s
probably software out there containing #define unused foo.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Use __unused0 instead of __unused for user visible struct member names
[not found] ` <20120103065659.GA32216-OoSGOWW0KRunlFQ6Q1D1Y0B+6BGkLq7r@public.gmane.org>
@ 2013-11-11 14:59 ` Thorsten Glaser
0 siblings, 0 replies; 11+ messages in thread
From: Thorsten Glaser @ 2013-11-11 14:59 UTC (permalink / raw)
To: Sam Ravnborg
Cc: Jonathan Nieder, Guillem Jover,
public-linux-kernel-u79uwXL29TY76Z2rM5mHXA-wOFGN7rlS/M9smdsby/KFg,
public-linux-kbuild-u79uwXL29TY76Z2rM5mHXA-wOFGN7rlS/M9smdsby/KFg,
public-linux-api-u79uwXL29TY76Z2rM5mHXA-wOFGN7rlS/M9smdsby/KFg,
Michal Marek, Justin Cormack
Sam Ravnborg dixit:
>> Guillem Jover wrote:
>> > On BSD systems __unused has traditionally been defined to mean the
>> > equivalent of gcc's __attribute__((__unused__)), some parts of the
[…]
>^__ is reserved for libc internal stuff and there is no reason to
>name the unused/padding members "__unused".
Considering that glibc has seen the light¹ now too, can we please
do something about these now? The BSD tools (not just NetBSD®) have
been using this for far longer after all…
Currently (git pull --ff torvalds master), we have:
• 2 occurrences of files *inside* the Linux kernel defining __unused
to² __attribute__((unused)) themselves
• 68 struct members and function arguments called __unused
>So one or a set of patches that rename them all to something more
>sensible would be fine.
I think __unused0 is okay as it matches current __unused[0-9] in
use by other parts of the Linux kernel – although glibc now uses
__glibc_reserved[0-9], I think this doesn’t look like the Linux
kernel should use it ☻☺
① http://thread.gmane.org/gmane.comp.lib.glibc.alpha/36439
② I’ve recently come to the belief that this should be
__attribute__((__unused__)) in all cases, i.e. all those
attribute namings need double underscores before and after,
as some software likes to #define printf to something else,
lighttpd does #define bounded something else, so there’s
probably software out there containing #define unused foo.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2013-11-11 15:11 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-02 2:44 [PATCH] Use __unused0 instead of __unused for user visible struct member names Guillem Jover
2012-01-02 20:22 ` Jonathan Nieder
2012-01-03 6:56 ` Sam Ravnborg
2012-01-03 6:56 ` Sam Ravnborg
2012-01-04 8:14 ` Guillem Jover
2012-01-04 11:03 ` Michal Marek
2012-01-04 13:35 ` Sam Ravnborg
2012-01-04 13:35 ` Sam Ravnborg
2013-11-11 14:59 ` Thorsten Glaser
2013-11-11 14:59 ` Thorsten Glaser
[not found] ` <20120103065659.GA32216-OoSGOWW0KRunlFQ6Q1D1Y0B+6BGkLq7r@public.gmane.org>
2013-11-11 14:59 ` Thorsten Glaser
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.