* Re: [PATCH] Use __unused0 instead of __unused for user visible struct member names [not found] <20120102024418.GA10483@gaara.hadrons.org> @ 2012-01-02 20:22 ` Jonathan Nieder 2012-01-03 6:56 ` Sam Ravnborg 0 siblings, 1 reply; 6+ 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] 6+ messages in thread
* Re: [PATCH] Use __unused0 instead of __unused for user visible struct member names 2012-01-02 20:22 ` [PATCH] Use __unused0 instead of __unused for user visible struct member names Jonathan Nieder @ 2012-01-03 6:56 ` Sam Ravnborg 2012-01-04 8:14 ` Guillem Jover 2013-11-11 14:59 ` Thorsten Glaser 0 siblings, 2 replies; 6+ 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] 6+ 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 2013-11-11 14:59 ` Thorsten Glaser 1 sibling, 1 reply; 6+ 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] 6+ 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; 6+ 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] 6+ messages in thread
* Re: [PATCH] Use __unused0 instead of __unused for user visible struct member names 2012-01-04 11:03 ` Michal Marek @ 2012-01-04 13:35 ` Sam Ravnborg 0 siblings, 0 replies; 6+ 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] 6+ 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 @ 2013-11-11 14:59 ` Thorsten Glaser 1 sibling, 0 replies; 6+ 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] 6+ messages in thread
end of thread, other threads:[~2013-11-11 15:11 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20120102024418.GA10483@gaara.hadrons.org>
2012-01-02 20:22 ` [PATCH] Use __unused0 instead of __unused for user visible struct member names Jonathan Nieder
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
2013-11-11 14:59 ` Thorsten Glaser
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox