mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 1/2] usb: gadget: print a meaningful error message
@ 2017-03-09  8:37 Sascha Hauer
  2017-03-09  8:37 ` [PATCH 2/2] usb: gadget: properly release f_multi_opts Sascha Hauer
  0 siblings, 1 reply; 2+ messages in thread
From: Sascha Hauer @ 2017-03-09  8:37 UTC (permalink / raw)
  To: Barebox List

We can only register one USB multi gadget, so check for it being
already registered and print a meaningful error message if it is.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/usb/gadget/multi.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/usb/gadget/multi.c b/drivers/usb/gadget/multi.c
index 13fa622f01..a2cfc8d850 100644
--- a/drivers/usb/gadget/multi.c
+++ b/drivers/usb/gadget/multi.c
@@ -234,6 +234,11 @@ static struct usb_composite_driver multi_driver = {
 
 int usb_multi_register(struct f_multi_opts *opts)
 {
+	if (gadget_multi_opts) {
+		pr_err("USB multi gadget already registered\n");
+		return -EBUSY;
+	}
+
 	gadget_multi_opts = opts;
 
 	return usb_composite_probe(&multi_driver);
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 2+ messages in thread

* [PATCH 2/2] usb: gadget: properly release f_multi_opts
  2017-03-09  8:37 [PATCH 1/2] usb: gadget: print a meaningful error message Sascha Hauer
@ 2017-03-09  8:37 ` Sascha Hauer
  0 siblings, 0 replies; 2+ messages in thread
From: Sascha Hauer @ 2017-03-09  8:37 UTC (permalink / raw)
  To: Barebox List

The usbgadget commands uses statically allocated f_multi_opts and passes
this to usb_multi_register(). These f_multi_opts are of course no
longer valid when we leave the usbgadget command. Luckily we do not use
the data after we left the usbgadget command, so this never has been a
problem. However, f_multi_opts has some allocated members which we can
not free anymore on gadget unregistration because we no longer have the
pointer to them.

Fix this by adding a release function to struct f_multi_opts. This way
we can allocate all memory dynamically and properly free it when not
used anymore.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 commands/usbgadget.c       | 19 +++++++++++++------
 drivers/usb/gadget/multi.c | 18 ++++++++++++++++--
 include/usb/gadget-multi.h |  2 ++
 3 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/commands/usbgadget.c b/commands/usbgadget.c
index a7e8d6c0c3..314884aee8 100644
--- a/commands/usbgadget.c
+++ b/commands/usbgadget.c
@@ -31,10 +31,10 @@
 
 static int do_usbgadget(int argc, char *argv[])
 {
-	int opt;
+	int opt, ret;
 	int acm = 1, create_serial = 0;
 	char *fastboot_opts = NULL, *dfu_opts = NULL;
-	struct f_multi_opts opts = {};
+	struct f_multi_opts *opts;
 
 	while ((opt = getopt(argc, argv, "asdA:D:")) > 0) {
 		switch (opt) {
@@ -73,19 +73,26 @@ static int do_usbgadget(int argc, char *argv[])
 		return -EINVAL;
 	}
 
+	opts = xzalloc(sizeof(*opts));
+	opts->release = usb_multi_opts_release;
+
 	if (fastboot_opts) {
-		opts.fastboot_opts.files = file_list_parse(fastboot_opts);
+		opts->fastboot_opts.files = file_list_parse(fastboot_opts);
 	}
 
 	if (dfu_opts) {
-		opts.dfu_opts.files = file_list_parse(dfu_opts);
+		opts->dfu_opts.files = file_list_parse(dfu_opts);
 	}
 
 	if (create_serial) {
-		opts.create_acm = acm;
+		opts->create_acm = acm;
 	}
 
-	return usb_multi_register(&opts);
+	ret = usb_multi_register(opts);
+	if (ret)
+		usb_multi_opts_release(opts);
+
+	return ret;
 }
 
 BAREBOX_CMD_HELP_START(usbgadget)
diff --git a/drivers/usb/gadget/multi.c b/drivers/usb/gadget/multi.c
index a2cfc8d850..6385c16186 100644
--- a/drivers/usb/gadget/multi.c
+++ b/drivers/usb/gadget/multi.c
@@ -246,8 +246,22 @@ int usb_multi_register(struct f_multi_opts *opts)
 
 void usb_multi_unregister(void)
 {
-	if (gadget_multi_opts)
-		usb_composite_unregister(&multi_driver);
+	if (!gadget_multi_opts)
+		return;
+
+	usb_composite_unregister(&multi_driver);
+	if (gadget_multi_opts->release)
+		gadget_multi_opts->release(gadget_multi_opts);
 
 	gadget_multi_opts = NULL;
 }
+
+void usb_multi_opts_release(struct f_multi_opts *opts)
+{
+	if (opts->fastboot_opts.files)
+		file_list_free(opts->fastboot_opts.files);
+	if (opts->dfu_opts.files)
+		file_list_free(opts->dfu_opts.files);
+
+	free(opts);
+}
diff --git a/include/usb/gadget-multi.h b/include/usb/gadget-multi.h
index 5ca462326a..81beddc7cf 100644
--- a/include/usb/gadget-multi.h
+++ b/include/usb/gadget-multi.h
@@ -9,9 +9,11 @@ struct f_multi_opts {
 	struct f_fastboot_opts fastboot_opts;
 	struct f_dfu_opts dfu_opts;
 	int create_acm;
+	void (*release)(struct f_multi_opts *opts);
 };
 
 int usb_multi_register(struct f_multi_opts *opts);
 void usb_multi_unregister(void);
+void usb_multi_opts_release(struct f_multi_opts *opts);
 
 #endif /* __USB_GADGET_MULTI_H */
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2017-03-09  8:38 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-09  8:37 [PATCH 1/2] usb: gadget: print a meaningful error message Sascha Hauer
2017-03-09  8:37 ` [PATCH 2/2] usb: gadget: properly release f_multi_opts Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox