[U-Boot] [PATCH 1/7] dfu:usb: Support for g_dnl composite download gadget.

[Mike Frysinger]

On Tuesday 03 July 2012 05:38:05 Lukasz Majewski wrote:
> --- /dev/null
> +++ b/drivers/usb/gadget/g_dnl.c
>
> +static const char shortname[] = "usb_dnl_";

shortname -> gadget_name_prefix

> +static void g_dnl_suspend(struct usb_composite_dev *cdev)
> +{
> +	if (cdev->gadget->speed == USB_SPEED_UNKNOWN)
> +		return;
> +
> +	debug("suspend\n");
> +}
> +
> +static void g_dnl_resume(struct usb_composite_dev *cdev)
> +{
> +	debug("resume\n");
> +}

do suspend/resume funcs make any sense in u-boot ?

> +int g_dnl_init(char *s)

const char

> +{
> +	int ret;
> +	static char str[16];
> +
> +	memset(str, '\0', sizeof(str));
> +
> +	strncpy(str, shortname, sizeof(shortname));

no need for the memset.  this strncpy looks broken -- the 3rd arg is for how 
many bytes are available in the *dest* buffer, not how long the source is.

> +	if (!strncmp(s, "dfu", sizeof(s))) {

sizeof() here is wrong -- that gives you back 4 (the size of a pointer) on 
32bit systems

> +		strncat(str, s, sizeof(str));

this is also incorrect.  the length given to strncat is how many bytes are 
left, not the total length.

since this string parsing logic is all just completely broken, i'd suggest 
replacing it all with:

{
	int ret;
	/* We only allow "dfu" atm, so 3 should be enough */
	static char name[sizeof(shortname) + 3];

	if (strcmp(s, "dfu")) {
		printf("%s: unknown command: %s\n", __func__, s);
		return -EINVAL;
	}

	strcpy(name, shortname);
	strcat(name, s);

> +	if (ret) {
> +		printf("%s: failed!, error:%d ", __func__, ret);

needs a space between "error:" and "%d"

> --- /dev/null
> +++ b/include/g_dnl.h
>
> +#include <linux/usb/ch9.h>
> +#include <usbdescriptors.h>
> +#include <linux/usb/gadget.h>

unused -> delete

> +int g_dnl_init(char *s);

int g_dnl_register(const char *type);

this also needs documentation in the header explaining how to use this func

> +void g_dnl_cleanup(void);

void g_dnl_unregister(void);

> +/* USB initialization declaration - board specific*/
> +void board_usb_init(void);

not used in these files -> delete
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120720/587193d8/attachment.pgp>