Re: coverity-desc-bitmap-overrun-fix-fix

[Ingo Oeser <ioe-lkml@rameria.de>]

[-- Attachment #1: Type: text/plain, Size: 1432 bytes --]

Hi Andrew,
hi lkml,

On Tuesday 28 June 2005 13:00, you wrote:
> From a maintainability POV it would be better to memset the whole array
> beforehand - I changed the patch to do that)
 
Yes, but then you should not assign 0xff to memset regions.

> Signed-off-by: Zaur Kambarov <zkambarov@coverity.com>
> Cc: <linux-usb-devel@lists.sourceforge.net?
> Cc: Greg KH <greg@kroah.com>
> Signed-off-by: Andrew Morton <akpm@osdl.org>
> ---
> 
>  drivers/usb/host/ohci-hub.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff -puN drivers/usb/host/ohci-hub.c~coverity-desc-bitmap-overrun-fix drivers/usb/host/ohci-hub.c
> --- 25/drivers/usb/host/ohci-hub.c~coverity-desc-bitmap-overrun-fix	2005-06-24 22:11:00.000000000 -0700
> +++ 25-akpm/drivers/usb/host/ohci-hub.c	2005-06-24 22:19:48.000000000 -0700
> @@ -419,10 +419,11 @@ ohci_hub_descriptor (
>  
>  	/* two bitmaps:  ports removable, and usb 1.0 legacy PortPwrCtrlMask */
>  	rh = roothub_b (ohci);
> +	memset(desc->bitmap, 0xff, sizeof(desc->bitmap));
>  	desc->bitmap [0] = rh & RH_B_DR;
>  	if (ports > 7) {
>  		desc->bitmap [1] = (rh & RH_B_DR) >> 8;
> -		desc->bitmap [2] = desc->bitmap [3] = 0xff;
> +		desc->bitmap [2] = 0xff;
>  	} else
>  		desc->bitmap [1] = 0xff;
>  }

I would suggest:

	if (ports > 7) 
		desc->bitmap[1] = (rh & RH_B_DR) >> 8

instead of the whole if construct.

Regards

Ingo Oeser


[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]