2007-01-18 15:10:48

by Robert P. J. Day

[permalink] [raw]
Subject: [PATCH] Centralize the macro definition of "__packed".


Centralize the attribute macro definition of "__packed" so no
subsystem has to do that explicitly.

Signed-off-by: Robert P. J. Day <[email protected]>

---

compile tested to make sure the HFS subsystem still builds. now
there's just 50 gazillion usages of "__attribute__((packed))" that can
be tightened up.


fs/hfs/hfs.h | 2 --
fs/hfsplus/hfsplus_raw.h | 2 --
include/linux/compiler-gcc.h | 1 +
3 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/fs/hfs/hfs.h b/fs/hfs/hfs.h
index 88099ab..1445e3a 100644
--- a/fs/hfs/hfs.h
+++ b/fs/hfs/hfs.h
@@ -83,8 +83,6 @@

/*======== HFS structures as they appear on the disk ========*/

-#define __packed __attribute__ ((packed))
-
/* Pascal-style string of up to 31 characters */
struct hfs_name {
u8 len;
diff --git a/fs/hfsplus/hfsplus_raw.h b/fs/hfsplus/hfsplus_raw.h
index 4920553..fe99fe8 100644
--- a/fs/hfsplus/hfsplus_raw.h
+++ b/fs/hfsplus/hfsplus_raw.h
@@ -15,8 +15,6 @@

#include <linux/types.h>

-#define __packed __attribute__ ((packed))
-
/* Some constants */
#define HFSPLUS_SECTOR_SIZE 512
#define HFSPLUS_SECTOR_SHIFT 9
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 6e1c44a..b6e39f5 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -27,6 +27,7 @@
#define __inline__ __inline__ __attribute__((always_inline))
#define __inline __inline __attribute__((always_inline))
#define __deprecated __attribute__((deprecated))
+#define __packed __attribute__((packed))
#define noinline __attribute__((noinline))
#define __attribute_pure__ __attribute__((pure))
#define __attribute_const__ __attribute__((__const__))


2007-01-18 16:26:26

by Robert P. J. Day

[permalink] [raw]
Subject: Re: [PATCH] Centralize the macro definition of "__packed".

On Thu, 18 Jan 2007, Robert P. J. Day wrote:

> Centralize the attribute macro definition of "__packed" so no
> subsystem has to do that explicitly.

ummm ... might want to ignore this submission, i want to do some
tweaking first. sorry.

rday

2007-01-18 16:48:55

by Tim Schmielau

[permalink] [raw]
Subject: Re: [PATCH] Centralize the macro definition of "__packed".

On Thu, 18 Jan 2007, Robert P. J. Day wrote:

> Centralize the attribute macro definition of "__packed" so no
> subsystem has to do that explicitly.
>
> Signed-off-by: Robert P. J. Day <[email protected]>
>
> ---
>
> compile tested to make sure the HFS subsystem still builds. now
> there's just 50 gazillion usages of "__attribute__((packed))" that can
> be tightened up.
>
>
> fs/hfs/hfs.h | 2 --
> fs/hfsplus/hfsplus_raw.h | 2 --
> include/linux/compiler-gcc.h | 1 +
> 3 files changed, 1 insertion(+), 4 deletions(-)

Moving definitions into compiler-gcc.h only will screw non-gcc compilers
like icc.
They should probably go into the generic section of compiler.h instead.

Tim

2007-01-18 16:59:43

by Robert P. J. Day

[permalink] [raw]
Subject: Re: [PATCH] Centralize the macro definition of "__packed".

On Thu, 18 Jan 2007, Tim Schmielau wrote:

> On Thu, 18 Jan 2007, Robert P. J. Day wrote:
>
> > Centralize the attribute macro definition of "__packed" so no
> > subsystem has to do that explicitly.
> >
> > Signed-off-by: Robert P. J. Day <[email protected]>
> >
> > ---
> >
> > compile tested to make sure the HFS subsystem still builds. now
> > there's just 50 gazillion usages of "__attribute__((packed))" that can
> > be tightened up.
> >
> >
> > fs/hfs/hfs.h | 2 --
> > fs/hfsplus/hfsplus_raw.h | 2 --
> > include/linux/compiler-gcc.h | 1 +
> > 3 files changed, 1 insertion(+), 4 deletions(-)
>
> Moving definitions into compiler-gcc.h only will screw non-gcc
> compilers like icc. They should probably go into the generic section
> of compiler.h instead.

actually, it *appears* that the standard works this way. the macro
"__deprecated" is defined in compiler-gcc.h with:

#define __deprecated __attribute__((deprecated))

while the more generic compiler.h handles whether or not it was
defined:

#ifndef __deprecated
# define __deprecated /* unimplemented */
#endif

so i'm guessing that's how any new attribute shortcut macros should be
handled, yes?

rday

2007-01-18 17:07:37

by Tim Schmielau

[permalink] [raw]
Subject: Re: [PATCH] Centralize the macro definition of "__packed".

On Thu, 18 Jan 2007, Robert P. J. Day wrote:

> actually, it *appears* that the standard works this way. the macro
> "__deprecated" is defined in compiler-gcc.h with:
>
> #define __deprecated __attribute__((deprecated))
>
> while the more generic compiler.h handles whether or not it was
> defined:
>
> #ifndef __deprecated
> # define __deprecated /* unimplemented */
> #endif
>
> so i'm guessing that's how any new attribute shortcut macros should be
> handled, yes?

Well, since the definitions lived well in compiler-generic land for quite
some time, I'd guess it should be ok not to #ifndef - guard them.
likely() and unlikely() are currently handled like that.
If the need ever arises to make them compiler specific, whoever does that
can still add the #ifndef then.

Tim

2007-01-18 17:16:21

by Robert P. J. Day

[permalink] [raw]
Subject: Re: [PATCH] Centralize the macro definition of "__packed".

On Thu, 18 Jan 2007, Tim Schmielau wrote:

> On Thu, 18 Jan 2007, Robert P. J. Day wrote:
>
> > actually, it *appears* that the standard works this way. the macro
> > "__deprecated" is defined in compiler-gcc.h with:
> >
> > #define __deprecated __attribute__((deprecated))
> >
> > while the more generic compiler.h handles whether or not it was
> > defined:
> >
> > #ifndef __deprecated
> > # define __deprecated /* unimplemented */
> > #endif
> >
> > so i'm guessing that's how any new attribute shortcut macros should be
> > handled, yes?
>
> Well, since the definitions lived well in compiler-generic land for
> quite some time, I'd guess it should be ok not to #ifndef - guard
> them. likely() and unlikely() are currently handled like that. If
> the need ever arises to make them compiler specific, whoever does
> that can still add the #ifndef then.

as it is, i believe the only two compilers that are officially
supported for building the kernel are gcc and icc, and icc identifies
itself as a GNU compiler anyway, so adding to compiler-gcc.h should be
safe until the situation changes.

and, as you say, if the situation changes, fixing compiler.h would be
easy.

rday