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

From: Oliver Neukum
Date: Fri Aug 29 2008 - 03:28:48 EST


Am Freitag 29 August 2008 01:00:31 schrieb greg@xxxxxxxxx:
> From: Takahiro Hirofuchi <hirofuchi@xxxxxxxxxxxxxxxxxxxxx>

> +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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/