Hello All,
I noticed a bug in the last patch I sent. Please see inline.
The corrected patch is on its way.
On Wednesday, March 07, 2012 9:30 AM Andrzej Pietrasiewicz wrote:
<snip>
> diff --git a/drivers/usb/gadget/g_ffs.c b/drivers/usb/gadget/g_ffs.c
<snip>
> static int gfs_bind(struct usb_composite_dev *cdev)
> {
> int ret, i;
>
> ENTER();
>
> - if (WARN_ON(!gfs_ffs_data))
> + if (missing_funcs)
> return -ENODEV;
>
> ret = gether_setup(cdev->gadget, gfs_hostaddr);
> if (unlikely(ret < 0))
> goto error_quick;
> + gfs_ether_setup = true;
>
> ret = usb_string_ids_tab(cdev, gfs_strings);
> if (unlikely(ret < 0))
> goto error;
>
> - ret = functionfs_bind(gfs_ffs_data, cdev);
> - if (unlikely(ret < 0))
> - goto error;
> + for (i = func_num; --i; ) {
> + ret = functionfs_bind(ffs_tab[i].ffs_data, cdev);
> + if (unlikely(ret < 0)) {
> + while (++i < func_num)
> + functionfs_unbind(ffs_tab[i].ffs_data);
> + goto error;
> + }
> + }
The for loop counter is decremented in the loop condition, which is good,
because the maximum possible index into ffs_tab is, or course, func_num - 1.
But the counter is pre-decremented, which is bad, because the the
functionfs_bind call will never be executed for function number 0.
<snip>
> @@ -266,22 +409,29 @@ static int gfs_unbind(struct usb_composite_dev
*cdev)
> * from composite on orror recovery, but what you're gonna
> * do...?
> */
> - if (gfs_ffs_data) {
> + if (gfs_ether_setup)
> gether_cleanup();
> - functionfs_unbind(gfs_ffs_data);
> - gfs_ffs_data = NULL;
> - }
> + gfs_ether_setup = false;
> +
> + for (i = func_num; --i; )
> + if (ffs_tab[i].ffs_data)
> + functionfs_unbind(ffs_tab[i].ffs_data);
>
The same here with functionfs_unbind.
<snip>
Andrzej
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
