| Message ID | 3a0a3ebfe65b7cc5b413ecc11af7aa9b0c97a532.1551253561.git.mirq-linux@rere.qmqm.pl (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: gadget: u_serial: console for multiple ports | expand |
On Wed, Feb 27, 2019 at 08:48:57AM +0100, Michał Mirosław wrote: > Prevent OBEX serial port from ever becoming a console. Why? > /* management of individual TTY ports */ > -int gserial_alloc_line(unsigned char *port_line); > +int gserial_alloc_line(unsigned char *port_line, bool maybe_console); Boolean flags in function calls are a major pain over time. There is no way to know, when you see this function being called, what "true" or "false" means, so you need to go look up the function header here (as I had to do in this patch), to get a clue as to what is going on. It is MUCH better to just have a new function: gserial_alloc_line_no_console() and leave gserial_alloc_line() alone, or, just have two functions: gserial_alloc_line_no_console() gserial_alloc_line_console() which resolve internally to the same function: static int __gserial_alloc_line(unsigned char *port_line, bool maybe_console) { ... } int gserial_alloc_line_no_console(unsigned char *port_line) { return __gserial_alloc_line(port_line, false); } int gserial_alloc_line_console(unsigned char *port_line) { return __gserial_alloc_line(port_line, true); } Trust me, when you have to go fix this code up in 5 years, you will thank me for having to force you to write the code this way :) thanks, greg k-h
diff --git a/drivers/usb/gadget/function/f_acm.c b/drivers/usb/gadget/function/f_acm.c index 9fc98de83624..60d18c7dcef3 100644 --- a/drivers/usb/gadget/function/f_acm.c +++ b/drivers/usb/gadget/function/f_acm.c @@ -807,7 +807,7 @@ static struct usb_function_instance *acm_alloc_instance(void) if (!opts) return ERR_PTR(-ENOMEM); opts->func_inst.free_func_inst = acm_free_instance; - ret = gserial_alloc_line(&opts->port_num); + ret = gserial_alloc_line(&opts->port_num, true); if (ret) { kfree(opts); return ERR_PTR(ret); diff --git a/drivers/usb/gadget/function/f_obex.c b/drivers/usb/gadget/function/f_obex.c index 55b7f57d2dc7..8242ba76dc1e 100644 --- a/drivers/usb/gadget/function/f_obex.c +++ b/drivers/usb/gadget/function/f_obex.c @@ -432,7 +432,7 @@ static struct usb_function_instance *obex_alloc_inst(void) return ERR_PTR(-ENOMEM); opts->func_inst.free_func_inst = obex_free_inst; - ret = gserial_alloc_line(&opts->port_num); + ret = gserial_alloc_line(&opts->port_num, false); if (ret) { kfree(opts); return ERR_PTR(ret); diff --git a/drivers/usb/gadget/function/f_serial.c b/drivers/usb/gadget/function/f_serial.c index c860f30a0ea2..788179a9a23c 100644 --- a/drivers/usb/gadget/function/f_serial.c +++ b/drivers/usb/gadget/function/f_serial.c @@ -303,7 +303,7 @@ static struct usb_function_instance *gser_alloc_inst(void) return ERR_PTR(-ENOMEM); opts->func_inst.free_func_inst = gser_free_inst; - ret = gserial_alloc_line(&opts->port_num); + ret = gserial_alloc_line(&opts->port_num, true); if (ret) { kfree(opts); return ERR_PTR(ret); diff --git a/drivers/usb/gadget/function/u_serial.c b/drivers/usb/gadget/function/u_serial.c index 8d2d861e1543..dd138d372940 100644 --- a/drivers/usb/gadget/function/u_serial.c +++ b/drivers/usb/gadget/function/u_serial.c @@ -1179,7 +1179,7 @@ void gserial_free_line(unsigned char port_num) } EXPORT_SYMBOL_GPL(gserial_free_line); -int gserial_alloc_line(unsigned char *line_num) +int gserial_alloc_line(unsigned char *line_num, bool maybe_console) { struct usb_cdc_line_coding coding; struct gs_port *port; @@ -1221,7 +1221,7 @@ int gserial_alloc_line(unsigned char *line_num) } *line_num = port_num; - if (!port_num) + if (maybe_console && !port_num) gs_console_init(port); err: return ret; diff --git a/drivers/usb/gadget/function/u_serial.h b/drivers/usb/gadget/function/u_serial.h index 9acaac1cbb75..40e89ddfe90e 100644 --- a/drivers/usb/gadget/function/u_serial.h +++ b/drivers/usb/gadget/function/u_serial.h @@ -54,7 +54,7 @@ struct usb_request *gs_alloc_req(struct usb_ep *ep, unsigned len, gfp_t flags); void gs_free_req(struct usb_ep *, struct usb_request *req); /* management of individual TTY ports */ -int gserial_alloc_line(unsigned char *port_line); +int gserial_alloc_line(unsigned char *port_line, bool maybe_console); void gserial_free_line(unsigned char port_line); /* connect/disconnect is handled by individual functions */ diff --git a/drivers/usb/gadget/legacy/dbgp.c b/drivers/usb/gadget/legacy/dbgp.c index e1d566c9918a..a6785a4a1ac6 100644 --- a/drivers/usb/gadget/legacy/dbgp.c +++ b/drivers/usb/gadget/legacy/dbgp.c @@ -305,7 +305,7 @@ static int dbgp_bind(struct usb_gadget *gadget, goto fail; } - if (gserial_alloc_line(&tty_line)) { + if (gserial_alloc_line(&tty_line, true)) { stp = 4; err = -ENODEV; goto fail;
Prevent OBEX serial port from ever becoming a console. Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl> --- drivers/usb/gadget/function/f_acm.c | 2 +- drivers/usb/gadget/function/f_obex.c | 2 +- drivers/usb/gadget/function/f_serial.c | 2 +- drivers/usb/gadget/function/u_serial.c | 4 ++-- drivers/usb/gadget/function/u_serial.h | 2 +- drivers/usb/gadget/legacy/dbgp.c | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-)