Re: [linux-usb-devel] [PATCH RFC] ZyDAS ZD1211 USB-WLAN driver

[Daniel Drake <dsd@gentoo.org>]

Oliver Neukum wrote:
> +static int read_mac_addr(struct zd_chip *chip, u8 *mac_addr)
> +{
> +	static const zd_addr_t addr[2] = { CR_MAC_ADDR_P1, CR_MAC_ADDR_P2 };
> +	return _read_mac_addr(chip, mac_addr, (const zd_addr_t *)addr);
> +}
> 
> Why on the stack?

Technically it's not on the stack because it is static const (it goes in 
rodata), but I don't think that this invalidates your point. What's the 
alternative? kmalloc and kfree every time?

(Just seems a little over the top for such a small array)

> +static int zd1211_hw_reset_phy(struct zd_chip *chip)
> +{
> +	static const struct zd_ioreq16 ioreqs[] = {
> 
> This is too much to allocate on the stack.

Again static const, it's definately in rodata, checked with objdump. Do 
we need to change this?

> +static void disconnect(struct usb_interface *intf)
> This is racy. It allows io to disconnected devices. You must take the
> lock and set a flag that you test after you've taken the lock elsewhere.

Will fix, thanks.

Daniel