From: Oliver Neukum <oliver@neukum.org>
Organization: NOvell
To: greg@kroah.com
Subject: Re: [patch 01/03] USB: USB/IP: add common functions needed
Date: Fri, 29 Aug 2008 09:29:51 +0200
User-Agent: KMail/1.9.9
Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
       Brian Merrell <bgmerrell@novell.com>,
       Takahiro Hirofuchi <hirofuchi@users.sourceforge.net>, gregkh@suse.de
References: <20080828225248.758800945@mini.kroah.org> <20080828230031.GB15014@kroah.com>
In-Reply-To: <20080828230031.GB15014@kroah.com>
MIME-Version: 1.0
Content-Type: text/plain;
  charset="iso-8859-1"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
Message-Id: <200808290929.52547.oliver@neukum.org>
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1823
Lines: 63

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