Re: Kingsun KS-959 USB IrDA dongle - release?candidate for submission (try 1)

Samuel Ortiz <[email protected]>
Newsgroups gmane.linux.irda.general,gmane.linux.usb.devel
Message-ID <[email protected]>
On Fri, Jul 20, 2007 at 04:58:29PM -0700, Pete Zaitcev wrote:
> On Sat, 21 Jul 2007 02:04:34 +0300, Samuel Ortiz <[email protected]> wrote:
> > On Sat, Jul 21, 2007 at 12:04:27AM +0200, Oliver Neukum wrote:
> 
> > > > +struct ks959_speedparams {
> > > > +	__le32 baudrate;		/* baud rate, little endian */
> > > > +	unsigned int data_bits : 2;	/* data bits - 5 (5..8) */
> > > > +	unsigned int : 1;
> > > > +	unsigned int stop_bits : 1;
> > > > +	unsigned int parity_enable : 1;
> > > > +	unsigned int parity_type : 1;
> > > > +	unsigned int : 1;
> > > > +	unsigned int reset : 1;
> > > > +	__u8 reserved[3];
> > > > +} __attribute__ ((packed));
> > > 
> > > The attribute is not needed.
> 
> > The driver uses sizeof(struct ks959_speedparams) in a couple places in
> > the code. If you don't pack it, the size will vary with different
> > architectures since it's not 32 bits aligned, so I think this attribute
> > is correct.
> 
> Oliver is correct, everything is perfectly aligned and the attribute
> is unnecessary.
Argghh, stupid me, should learn counting or something...the bit fields
take 1 byte, so the structure is indeed aligned.
So, yes, the packed attribute is not needed regardless of the compiler
reordering the bit fields or not.

> However the question is moot, since the bit fields vary in order.
> Bit fields must not be used, and shifts and masks must be used instead.
I didn't know about the bit field reordering, but it seems we should use
an __u8 and shift bits on it, yes.

Cheers,
Samuel.


> -- Pete
> 
> -------------------------------------------------------------------------
> This SF.net email is sponsored by: Microsoft
> Defy all challenges. Microsoft(R) Visual Studio 2005.
> http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
> _______________________________________________
> irda-users mailing list
> [email protected]
> http://lists.sourceforge.net/lists/listinfo/irda-users


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.