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

Attachment: signature.asc
Description: This is a digitally signed message part.

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to