回复: PR 1098: "struct ipovly" not packed.

[Denny.Bai]

typedef struct {
   long  l1;
   short s1;
   long  l2;
} s2_t __attribute__((packed));  /* does nothing */

--------------------------------------------------

if above code changed like this, "__attribute__((packed))" will work
and the warning will not appear again.

typedef struct {
   long  l1;
   short s1;
   long  l2;
}__attribute__((packed)) s2_t;


2006/7/6, Jay Monkman <jtm at smoothsmoothie.com>:
> Ralf Corsepius wrote:
> > On Sat, 2006-06-24 at 14:50 +1000, Chris Johns wrote:
> >> This PR:
> >>
> >> http://www.rtems.com/cgi-bin/gnatsweb.pl?cmd=view&database=RTEMS&pr=1098
> >>
> >> contains a patch related to this thread:
> >>
> >> http://www.rtems.org/ml/rtems-users/2006/may/msg00060.html
> >>
> >> It requires the packing of "struct ipovly" and "struct tcpiphdr".
> >>
> >> I would like to commit this patch and close the PR but before I do this
> >> I would like to know why this patch is needed for a specific ARM board
> >> and network device and not others.
> > And I would like to know why this patch is needed at all.
> >
> > 1. I fail to understand why this patch is required at all, because I
> > don't see why __attribute__(packed) should have any influence on the
> > structs you are trying to pack.
> >
> > 2. __attribute__(packed) is hack and should not be used.
> >
> > Can you provide an isolated example exposing the problem (Which?) you
> > are trying to solve?
> >
> > Ralf
> >
> >
>
> Sorry for the late response - I just saw this related to arm.
>
> That patch doesn't what it's supposed to. At least not with my compiler -
> gcc version 4.0.1 (RTEMS gcc-4.0.1-20050810/newlib-1.13.0-20050810-4)
>
> Putting __attribute__((packed)) at the end of struct doesn't pack the
> structure.
> I assume that it would cause an array of odd sized structs to be packed, but
> I'm
> not sure.
>
> If you want the structure to be packed, you need to put
> __attribute__((packed))
> on each element. At least that's my understanding. See my test at the end of
> this email
>
> I don't understand how this patch would help anyone, since I don't see how
> it
> would change the generated object code.
>
> Networking has been working on ARM for quite a while now. At least in 4.6
> and
> the 4.7 branch. (I haven't tested the latest code in the 4.7 branch, but as
> of a
> month ago it worked.)
>
> I'm not 100% convinced that the network code is bug-free on ARM because the
> stack doesn't align data nicely, but I know that it works for the common
> cases.
> It's been tested with millions of packets on hundreds of devices with no
> problems.
>
>
>
> I do disagree that __attribute__((packed)) should never be used. Without
> something like that, there's no guarantee how the compiler will align the
> data.
> I've seen it use two byte alignment on CPUs like x86. When we first started
> trying to get the network stack working on ARM we had to fix several of the
> problems.
>
> Do you know of a better alternative?
>
>
>
> ----------------------------------------------------------------------------
> Here's some example source code with different uses of
> __attribute__((packed))
> and the generated output (arm-rtems4.7-gcc -O1 -s test.c).
>
> I actually get a warning about __attribute__((packed)) being ignored for the
> struct where it comes at the end (s2_t).
>
>
>
> ----------------------------------------------------------------------------
> Test code:
> ----------------------------------------------------------------------------
> typedef struct {
>     long  l1;
>     short s1;
>     long  l2;
> } s1_t;
>
> typedef struct {
>     long  l1;
>     short s1;
>     long  l2;
> } s2_t __attribute__((packed));  /* does nothing */
>
> typedef struct {
>     long  l1 __attribute__((packed));
>     short s1 __attribute__((packed));
>     long  l2 __attribute__((packed));
> } s3_t;
>
>
> extern void func(void *);
>
> void test1(void)
> {
>
>     s1_t *s1 = (s1_t *)0x0;
>
>     func(s1);
>     func(&s1->l2);
> }
>
> void test2(void)
> {
>
>     s2_t *s2 = (s2_t *)0x0;
>
>     func(s2);
>     func(&s2->l2);
> }
>
> void test3(void)
> {
>
>     s3_t *s3 = (s3_t *)0x0;
>
>     func(s3);
>     func(&s3->l2);
> }
>
> ----------------------------------------------------------------------------
> Generated assembly (abridged)
> ----------------------------------------------------------------------------
> test1:
> 	str	lr, [sp, #-4]!
> 	mov	r0, #0
> 	bl	func
> 	mov	r0, #8      @ Element s1->l2 at offset of 8 - not packed
> 	bl	func
> 	ldr	pc, [sp], #4
>
> test2:
> 	str	lr, [sp, #-4]!
> 	mov	r0, #0
> 	bl	func
> 	mov	r0, #8      @ Element s2->l2 at offset of 8 - not packed
> 	bl	func
> 	ldr	pc, [sp], #4
>
> test3:
> 	str	lr, [sp, #-4]!
> 	mov	r0, #0
> 	bl	func
> 	mov	r0, #6      @ Element s3->l2 at offset of 6 - packed
> 	bl	func
> 	ldr	pc, [sp], #4
>
>


-- 
Bai.Shaolin
Northeastern  University, ShenYang,China