Am 08.08.2009 um 14:28 schrieb Paul Goyette:
On Sat, 8 Aug 2009, Marc Balmer wrote:The attached patch implements the mode_select operation for atapi tape drives. It is enabled only via a new quirk setting, so existing drives that haven't explicitly been determined to support this op won't even attempt it. (This is essentially the same patch as in PR kern/34832.) I've been running this in my own systems for about 3 1/2 years now, with my Seagate STT3401A (Travan 20/40GB) drive. I'll submit a separate patch later with a cleaned-up quick setting for this drive. This patch is actually required for the above tape drive to operate properly under NetBSD. The drive supports both 512-byte and 1024-byte block sizes, but defaults to 1024-bytes. Without this patch there it isn't possible to write 512-byte records to the tape!Any objections to committing this?Why do you use memset(&atapi_select, 0, atapi_select_len); instead of memset(&atapi_select, 0, sizeof(atapi_select)); and zap the u_int atapi_select_len variable?Good question!Basically, this whole routine was copied almost verbatim from the equivalent code in sys/dev/scsipi/st_scsi.c which also calculates the number of bytes to clear. Since it worked, I never looked any further.There is always the chance that the computed value of atapi_select_len (based on the _actual_ size of the device's page_0) is less than the possible maximum size of page_0. So we could save a few nanoseconds by clearing fewer bytes. In practice I don't expect this to make any real difference.
And on the worse side of possibilities, there is the chance of buffer overflow, if for some the reason the calculated value is bigger than the structs sice. This code should really use sizeof, to be unambigously clear.
If I change my patch, I'd want to update the calculation in st_scsi.c as well, to keep them in sync.
Yes, it should be changed there, too.
Thanks for the quick review! -------------------------------------------------------------------------| Paul Goyette | PGP DSS Key fingerprint: | E-mail addresses: | | Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com | | Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net | | Kernel Developer | | pgoyette at netbsd.org |-------------------------------------------------------------------------