Re: [patch 01/03] USB: USB/IP: add common functions needed

[Oliver Neukum <oliver@neukum.org>]

Am Freitag 29 August 2008 01:00:31 schrieb greg@kroah.com:
> From: Takahiro Hirofuchi <hirofuchi@users.sourceforge.net>

> +static ssize_t store_flag(struct device *dev, struct device_attribute *attr,
> +		const char *buf, size_t count)
> +{
> +	unsigned long flag;
> +
> +	sscanf(buf, "%lx", &flag);

Needs error handling

> +	usbip_debug_flag = flag;
> +
> +	return count;
> +}
> +DEVICE_ATTR(usbip_debug, (S_IRUGO | S_IWUSR), show_flag, store_flag);
> +
> +static void usbip_dump_buffer(char *buff, int bufflen)

Such functions better print into a buffer, handed over in one go.
Otherwise you can get results
-second message-
like this

> +/* there may be more cases to tweak the flags. */
> +static unsigned int tweak_transfer_flags(unsigned int flags)
> +{
> +
> +	if (flags & URB_NO_TRANSFER_DMA_MAP)
> +		/*
> +		 * vhci_hcd does not provide DMA-mapped I/O. The upper
> +		 * driver does not need to set this flag. The remote
> +		 * usbip.ko does not still perform DMA-mapped I/O for
> +		 * DMA-caplable host controllers. So, clear this flag.
> +		 */
> +		flags &= ~URB_NO_TRANSFER_DMA_MAP;
> +
> +	if (flags & URB_NO_SETUP_DMA_MAP)
> +		flags &= ~URB_NO_SETUP_DMA_MAP;

There's no need to do this conditionally.

> +/* must free buffer */
> +void *usbip_alloc_iso_desc_pdu(struct urb *urb, ssize_t *bufflen)
> +{
> +	void *buff;
> +	struct usbip_iso_packet_descriptor *iso;
> +	int np = urb->number_of_packets;
> +	ssize_t size = np * sizeof(*iso);
> +	int i;
> +
> +	buff = kzalloc(size, GFP_KERNEL);

Are you sure you cannot badly recourse here?

	Regards
		Oliver