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
the constant used in the memcpy() has some "interesting" implications.
:) ---------------------------------------------------------------------- | Paul Goyette | PGP DSS Key fingerprint: | E-mail addresses: | | Customer Service | FA29 0E3B 35AF E8AE 6651 | paul%whooppee.com@localhost | | Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette%juniper.net@localhost | ----------------------------------------------------------------------