On Sun, Jan 06, 2008 at 06:08:43AM -0800, Paul Goyette wrote: > On Sun, 6 Jan 2008, John Nemeth wrote: > > >On May 29, 5:30am, David Laight wrote: > >} On Sun, Jan 06, 2008 at 03:41:49AM -0700, John R. Shannon wrote: > >} > Line 1124 has: > >} > > >} > memcpy(inqbuf->vendor, "ADAPTEC ACB-4000 ", 28); > >} > > >} > and line 1144 has: > >} > > >} > memcpy(inqbuf->vendor, "EMULEX MT-02 QIC ", 28); > >} > > >} > yet inqbuf->vendor is declared in struct scsipi_inquiry_data as: > >} > > >} > char vendor[8]; > >} > >} and is followed by: > >} char product[16]; > >} char revision[4]; > >} so the memcpy updates all 3 fields :-) > > > > That is extremely grotty code! > > Shouldn't we at least replace the constant 28 with a macro that gives a > hint of what's going on? > > #define INQBUF_TRIPLET_SIZE (sizeof(inqbuf->vendor) + \ > sizeof(inqbuf->product) + \ > sizeof(inqbuf->revision)) > > It's still going to be grotty code, and won't address the issue of a > "perverse introduction of gratuitous padding" mentioned in another > message in this thread, but at least a reader would have some idea that A "gratuitous padding" here would break anyway, as this structure is filled in by hardware (it's what's returned by a device to an INQUIRY command). -- Manuel Bouyer <bouyer%antioche.eu.org@localhost> NetBSD: 26 ans d'experience feront toujours la difference --