On 09/15/2015 04:26 PM, Robert Baldyga wrote:
Since ep->driver_data is not used for endpoint claiming, neither for enabled/disabled state storing, we can reduce number of places where we read or modify it's value, as now it has no particular meaning for function or framework logic. In case of f_ecm, ep->driver_data was used only for endpoint claiming and marking endpoints as enabled, so we can simplify code by reducing it. Signed-off-by: Robert Baldyga <r.baldyga@xxxxxxxxxxx>
( ... )
@@ -820,14 +811,6 @@ fail: usb_ep_free_request(ecm->notify, ecm->notify_req); } - /* we might as well release our claims on endpoints */ - if (ecm->notify) - ecm->notify->driver_data = NULL; - if (ecm->port.out_ep) - ecm->port.out_ep->driver_data = NULL; - if (ecm->port.in_ep) - ecm->port.in_ep->driver_data = NULL; - ERROR(cdev, "%s: can't bind, err %d\n", f->name, status); return status;
You have done this in almost all functions but personally I'm really concern about this change.
By convention function should free all allocated resources when exiting with non 0 code. Endpoints are some kind of resources, they are "allocated" using usb_ep_autoconfig() and if you are not going to use them because error occurred you should free the using usb_ep_autoconfig_release(). Moreover, you have done this in source sink function so why not do this in all other?
Best regards, -- Krzysztof Opasiak Samsung R&D Institute Poland Samsung Electronics -- 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
