[U-Boot] [PATCH 4/4] USB: Staticize usb_storage.c

Mike Frysinger
2012-02-14T05:37:24Z

On Monday 13 February 2012 23:58:19 Marek Vasut wrote:
> Also, make usb_stor_buf local.

probably should get split out into its own patch

> --- a/common/usb_storage.c
> +++ b/common/usb_storage.c
>
> int usb_stor_scan(int mode)
> ...
> unsigned char i;
> struct usb_device *dev;
>
> - /* GJ */
> - memset(usb_stor_buf, 0, sizeof(usb_stor_buf));
> -
> if (mode == 1)
> printf(" scanning bus for storage devices... ");
> ...
> -int usb_stor_get_info(struct usb_device *dev, struct us_data *ss,
> +static int usb_stor_get_info(struct usb_device *dev, struct us_data *ss,
> block_dev_desc_t *dev_desc)
> {
> unsigned char perq, modi;
> unsigned long cap[2];
> unsigned long *capacity, *blksz;
> ccb *pccb = &usb_ccb;
> + unsigned char usb_stor_buf[512];
> +
> + /* GJ */
> + memset(usb_stor_buf, 0, sizeof(usb_stor_buf));

before, the code would do:
usb_stor_scan() {
memset(usb_stor_buf)
for (...) {
usb_stor_get_info() {
... use usb_stor_buf ...
}
}
}

now the new code calls memset multiple times. in a cursory reading of the
code, i'd say just drop the memset altogether. the get_info func will return
if the usb_inquiry call failed and the buffer contents don't matter. if
usb_query worked, then the buffer should contain valid data and the memset was
pointless.

looking a bit more, i think 512 is too big. it makes sense when it's a global
dumping ground that random parts of the code would use. but this buffer is
only used with usb_inquiry (which in reality is the SCSI INQUIRY command), and
that should require just 36 bytes.

so my suggestion would be:
- drop usb_stor_buf changes from this patch and make a dedicated one
- drop the useless GJ comment
- drop the useless memset
- shrink the buffer to 36 bytes
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120214/08b18e72/attachment.pgp>