Dear Michal Nazarewicz
Thank you very much for your kindness.
And i also agree with your comments.
I wil try to change my commit-msg & re-send it as soon as possible.
And about your last commets,
> Also, like written previously, I think there might be memory leak in
other error
> recovery paths as well. If you could take a look whether I'm right,
that would be
> awesome.
I will try to find out if the memory leaks in other error recovery paths
or not,
but i think that might be another case and need different patch-set.
(If i find out that, i will try to make another patch-set and send it
all again)
Best regards.
Yongsul Oh
On 2012년 03월 16일 20:14, Michal Nazarewicz wrote:
On Fri, 16 Mar 2012 07:27:15 +0100, Yongsul Oh
<yongsul96.oh@xxxxxxxxxxx> wrote:
In some usb gadget driver, (for example usb gadget serial
driver(serial.c),
s/usb/USB/g
s/gadget driver/composite gadget drivers/
I also don't think the list of examples is necessary here. Maybe just
a list file
names? My personal opinion, others may disagree though.
Also, comma should go after the closing paren.
multifunction composite driver(multi,c), nokia composite gadget
driver(nokia.c),
HID composite driver(hid.c), CDC composite driver(cdc2.c)..) the
configuration's
bind function by called the 'usb_add_config()' has multiple bind
config functions
s/by called the/called by the/
s/has/calls/
for each functionality. (for example cdc2 configuration bind function
-'cdc_do_config()'
s/functions for each functionality/functions, one for each USB
functionality/
has two functionality bind config functions -'ecm_bind_config()' &
'acm_bind_config()'
in CDC composite driver.)
In each functionality bind config function, new instance for each
functionality is
s/for each functionality//
allocated & initialized by 'kzalloc()' and finally the new instance
is added by
s/& initialized by 'kzalloc()' and finally the new instance/and/
'usb_add_function()'. After 'usb_add_function' state, already created
the instance
is only handled by it's configuration & freed from functionality
unbind function.
I'm not sure what you meant by this last sentence.
So, If an error occurred during the second functionality bind config
state,
s/If/if/
s/state//
(for example an error occurred at 'acm_bind_config()' after
succeeding of
'ecm_bind_function()') the created instance by 'acm_bind_config()'
s/the created instance by 'acm_bind_config()'/the instance created by
acm_bind_config()/
cannot be freed. And it makes memory leak situation.
s/freed. And it makes memory leak situation./freed creating a memory
leak./
This patch fixes this issue
s/issue/issue./
Also, drop apostrophes around function names. It looks weird.
Signed-off-by: Yongsul Oh <yongsul96.oh@xxxxxxxxxxx>
Acked-by: Michal Nazarewicz <mina86@xxxxxxxxxx>
Also, like written previously, I think there might be memory leak in
other error
recovery paths as well. If you could take a look whether I'm right,
that would be
awesome.
---
drivers/usb/gadget/composite.c | 13 +++++++++++++
1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/gadget/composite.c
b/drivers/usb/gadget/composite.c
index baaebf2..4cb1801 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -737,6 +737,19 @@ int usb_add_config(struct usb_composite_dev *cdev,
status = bind(config);
if (status < 0) {
+ while (!list_empty(&config->functions)) {
+ struct usb_function *f;
+
+ f = list_first_entry(&config->functions,
+ struct usb_function, list);
+ list_del(&f->list);
+ if (f->unbind) {
+ DBG(cdev, "unbind function '%s'/%p\n",
+ f->name, f);
+ f->unbind(config, f);
+ /* may free memory for "f" */
+ }
+ }
list_del(&config->list);
config->cdev = NULL;
} else {
--
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
