YOSHIFUJI Hideaki / 吉藤英明 <yoshfuji@linux-ipv6.org> wrote on 04/25/2008 
08:33:59 PM:

quoted text
> Please define __compat_sockaddr_storage{} with attribute((aligned(4))).
> If you do that we do not need attribute(packed).  No?

        I just tried it and the compiler seems to do that for this case,
but the gcc documentation says explicitly that you need both packed and
aligned if you want it to be a specific value. aligned(4) is a minimum
only and the compiler would be free to add pad still as long as the
padded value was also 4-byte aligned.
        In this case, we want no pad, so packed is the relevant part. If
had a char followed by a struct as in your previous example, the right
way to get 4-byte alignment is both aligned(4) and packed. Leaving off
packed would allow 8-byte or 16-byte (or 12 :-)) alignment, too.

quoted text
> 
> > +extern int compat_mc_setsockopt(struct sock *, int, int, char __user 
*, 
quoted text
> > int);
> > +
> 
> compat_do_inet_mc_setsockopt() or so.
> 
> >  #endif /* NET_COMPAT_H */
> > diff -ruNp linux-2.6.18.ppc64/net/compat.c 
> > linux-2.6.18.ppc64DLS1/net/compat.c
> > --- linux-2.6.18.ppc64/net/compat.c     2008-04-21 13:51:02.000000000 
> > -0700
> > +++ linux-2.6.18.ppc64DLS1/net/compat.c 2008-04-25 08:53:47.000000000 
> > -0700
> 
> net/compat.c is not the right place.
> Please put this in net/ipv4/compat.c or so.
> Or at least, these must be guarded by CONFIG_INET.

        This code is for both v4 and v6, so I'd rather it not be in
net/ipv4/compat.c. I didn't try without CONFIG_INET, -- will look
at that.

quoted text
> > +struct compat_group_filter {
> > +       __u32                            gf_interface;
> > +       struct __kernel_sockaddr_storage gf_group
> > +               __attribute__ ((aligned(4)));
> > +       __u32                            gf_fmode;
> > +       __u32                            gf_numsrc;
> > +       struct __kernel_sockaddr_storage gf_slist[1]
> > +               __attribute__ ((aligned(4)));
> > +} __attribute__ ((packed));
> > +
> 
> include/linux/in.n.

        I don't understand this comment. If you're suggesting I
put these in an include file, I didn't because this is the only
file that uses them and I don't want them to be visible to any
other code. If you mean something else, please explain.

quoted text
> 
> __copy_in_user().
> Maybe we can use __copy_in_user for u32?

        I'll look at it.

                                thanks!

                                        +-DLS


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html