Re: Implement mode_select for atapi tape drives - round 4

[Manuel Bouyer <bouyer%antioche.eu.org@localhost>]

On Sun, Aug 09, 2009 at 08:37:29PM +0100, David Laight wrote:
> On Sun, Aug 09, 2009 at 01:36:02PM +0200, Manuel Bouyer wrote:
> > On Sun, Aug 09, 2009 at 09:29:05AM +0100, Iain Hibbert wrote:
> > > On Sat, 8 Aug 2009, Paul Goyette wrote:
> > > 
> > > > Further comment/criticism is certainly welcomed.  In the absence of any
> > > > objection, I'd like to commit this in the next 7 to 10 days.
> > > 
> > > +/*
> > > + * Send a filled out parameter structure to the drive to
> > > + * set it into the desire modes etc.
> > > + */
> > > +int
> > > +st_mode_select(struct st_softc *st, int flags)
> > > +{
> > > + u_int select_len;
> > > + struct select {
> > > +         struct scsi_mode_parameter_header_6 header;
> > > +         struct scsi_general_block_descriptor blk_desc;
> > > +         u_char sense_data[MAX_PAGE_0_SIZE];
> > > + } select;
> > > + struct scsipi_periph *periph = st->sc_periph;
> > > +
> > > + select_len = sizeof(select.header) + sizeof(select.blk_desc) +
> > > +              st->page_0_size;
> > > 
> > > should select be __packed?
> > 
> > Yes, it should probably be. I know most hardware structures are not
> > declared __packed in the scsipi code, we've been luky this didn't cause
> > issues for now ...
> 
> Actually it probably shouldn't!
> You may want to define the alignment restriction of some of the members,
> and possibly of the structure itself.
> (ie if there are any 32bit items that are on odd 16bit boundaries, or
> where the entire structure isn't a multiple of 4 bytes - for systems
> where the ABI forces all structures to have 32bit alignment.)
> 
> Forcing 'packed' is overkill and will lead to very slow code in
> unnecessary places on systems that cannot do misaligned transfers in
> hardware.

I don't get it. If the hardware (scsi devices in this case) defines
a structure like e.g
struct foo {
        int8_t data1;
        int32_t data2;
        int8_t data3
        int32_t data4;
};
the software on the host side will have to do misaligned accesses whatever
we do, isn't it ? How do we avoid the compiler from padding the members
without __packed ?

> 
> Using structures to map hardware structures implicitly assumes knowledge
> of the way compilers lay out structures - which may, or may not, be
> guaranteed by the C standard.

in such a case I don't think we can allow the compiler to decide how to
lay out the structure.

-- 
Manuel Bouyer <bouyer%antioche.eu.org@localhost>
     NetBSD: 26 ans d'experience feront toujours la difference
--