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

[Mike Frysinger]

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>