Re: [PATCH v2 1/2] usb: gadget: initialize serialnumber
On Thu, Oct 26, 2023 at 04:43:24PM +0200, Marco Felsch wrote: > Windows hosts do require the serial number to be set to any ascii string > to enumerate correctly. Set the serial number if provided or to "unset" > if not to provide a sane default which works for both hosts. > > Reported-by: Andrey Zhizhikin > Signed-off-by: Marco Felsch > --- > Changelog > > v2: > - adapt commit message > - use barebox_get_serial_number() and "unset" > Applied, thanks Sascha > drivers/usb/gadget/udc/core.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c > index b58498680ad1..e7cfa0d5d836 100644 > --- a/drivers/usb/gadget/udc/core.c > +++ b/drivers/usb/gadget/udc/core.c > @@ -1204,7 +1204,7 @@ int usb_add_gadget(struct usb_gadget *gadget) > gadget->productname = xstrdup(barebox_get_model()); > dev_add_param_string(>dev, "productname", NULL, NULL, >>productname, NULL); > - gadget->serialnumber = xstrdup(""); > + gadget->serialnumber = xstrdup(barebox_get_serial_number() ? : "unset"); > dev_add_param_string(>dev, "serialnumber", NULL, NULL, >>serialnumber, NULL); > > -- > 2.39.2 > > > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [PATCH v2 1/2] usb: gadget: initialize serialnumber
On 27.10.23 10:19, Marco Felsch wrote: > On 23-10-27, Sascha Hauer wrote: >> On Fri, Oct 27, 2023 at 10:09:43AM +0200, Marco Felsch wrote: >>> On 23-10-27, Sascha Hauer wrote: On Thu, Oct 26, 2023 at 11:32:10PM +0200, Ahmad Fatoum wrote: > On 26.10.23 16:43, Marco Felsch wrote: >> Windows hosts do require the serial number to be set to any ascii string >> to enumerate correctly. Set the serial number if provided or to "unset" >> if not to provide a sane default which works for both hosts. >> >> Reported-by: Andrey Zhizhikin >> Signed-off-by: Marco Felsch > > Reviewed-by: Ahmad Fatoum > >> --- >> Changelog >> >> v2: >> - adapt commit message >> - use barebox_get_serial_number() and "unset" >> >> drivers/usb/gadget/udc/core.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/usb/gadget/udc/core.c >> b/drivers/usb/gadget/udc/core.c >> index b58498680ad1..e7cfa0d5d836 100644 >> --- a/drivers/usb/gadget/udc/core.c >> +++ b/drivers/usb/gadget/udc/core.c >> @@ -1204,7 +1204,7 @@ int usb_add_gadget(struct usb_gadget *gadget) >> gadget->productname = xstrdup(barebox_get_model()); >> dev_add_param_string(>dev, "productname", NULL, NULL, >> >productname, NULL); >> -gadget->serialnumber = xstrdup(""); >> +gadget->serialnumber = xstrdup(barebox_get_serial_number() ? : >> "unset"); > > Nitpick: xstrdup(NULL) == NULL, so the ternary could be moved out the > xstrdup. gadget->serialnumber is freed when the value of the variable is changed, so it must be an allocated string. gadget->serialnumber = "unset" would be wrong. >>> >>> I would have done the following: >>> >>> xstrdup(barebox_get_serial_number()) ? : xstrdup("unset"); >> >> Your original code looked better. This one looks like the ?: handles >> failures in xstrdup(). > > Right, thanks for the input. Agreed with Sascha, I forgot it had to be an allocated string. > > Regards, > Marco > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [PATCH v2 1/2] usb: gadget: initialize serialnumber
On 23-10-27, Sascha Hauer wrote: > On Fri, Oct 27, 2023 at 10:09:43AM +0200, Marco Felsch wrote: > > On 23-10-27, Sascha Hauer wrote: > > > On Thu, Oct 26, 2023 at 11:32:10PM +0200, Ahmad Fatoum wrote: > > > > On 26.10.23 16:43, Marco Felsch wrote: > > > > > Windows hosts do require the serial number to be set to any ascii > > > > > string > > > > > to enumerate correctly. Set the serial number if provided or to > > > > > "unset" > > > > > if not to provide a sane default which works for both hosts. > > > > > > > > > > Reported-by: Andrey Zhizhikin > > > > > Signed-off-by: Marco Felsch > > > > > > > > Reviewed-by: Ahmad Fatoum > > > > > > > > > --- > > > > > Changelog > > > > > > > > > > v2: > > > > > - adapt commit message > > > > > - use barebox_get_serial_number() and "unset" > > > > > > > > > > drivers/usb/gadget/udc/core.c | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/usb/gadget/udc/core.c > > > > > b/drivers/usb/gadget/udc/core.c > > > > > index b58498680ad1..e7cfa0d5d836 100644 > > > > > --- a/drivers/usb/gadget/udc/core.c > > > > > +++ b/drivers/usb/gadget/udc/core.c > > > > > @@ -1204,7 +1204,7 @@ int usb_add_gadget(struct usb_gadget *gadget) > > > > > gadget->productname = xstrdup(barebox_get_model()); > > > > > dev_add_param_string(>dev, "productname", NULL, NULL, > > > > >>productname, NULL); > > > > > - gadget->serialnumber = xstrdup(""); > > > > > + gadget->serialnumber = xstrdup(barebox_get_serial_number() ? : > > > > > "unset"); > > > > > > > > Nitpick: xstrdup(NULL) == NULL, so the ternary could be moved out the > > > > xstrdup. > > > > > > gadget->serialnumber is freed when the value of the variable is changed, > > > so it must be an allocated string. gadget->serialnumber = "unset" would > > > be wrong. > > > > I would have done the following: > > > > xstrdup(barebox_get_serial_number()) ? : xstrdup("unset"); > > Your original code looked better. This one looks like the ?: handles > failures in xstrdup(). Right, thanks for the input. Regards, Marco
Re: [PATCH v2 1/2] usb: gadget: initialize serialnumber
On Fri, Oct 27, 2023 at 10:09:43AM +0200, Marco Felsch wrote: > On 23-10-27, Sascha Hauer wrote: > > On Thu, Oct 26, 2023 at 11:32:10PM +0200, Ahmad Fatoum wrote: > > > On 26.10.23 16:43, Marco Felsch wrote: > > > > Windows hosts do require the serial number to be set to any ascii string > > > > to enumerate correctly. Set the serial number if provided or to "unset" > > > > if not to provide a sane default which works for both hosts. > > > > > > > > Reported-by: Andrey Zhizhikin > > > > Signed-off-by: Marco Felsch > > > > > > Reviewed-by: Ahmad Fatoum > > > > > > > --- > > > > Changelog > > > > > > > > v2: > > > > - adapt commit message > > > > - use barebox_get_serial_number() and "unset" > > > > > > > > drivers/usb/gadget/udc/core.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/usb/gadget/udc/core.c > > > > b/drivers/usb/gadget/udc/core.c > > > > index b58498680ad1..e7cfa0d5d836 100644 > > > > --- a/drivers/usb/gadget/udc/core.c > > > > +++ b/drivers/usb/gadget/udc/core.c > > > > @@ -1204,7 +1204,7 @@ int usb_add_gadget(struct usb_gadget *gadget) > > > > gadget->productname = xstrdup(barebox_get_model()); > > > > dev_add_param_string(>dev, "productname", NULL, NULL, > > > > >productname, NULL); > > > > - gadget->serialnumber = xstrdup(""); > > > > + gadget->serialnumber = xstrdup(barebox_get_serial_number() ? : > > > > "unset"); > > > > > > Nitpick: xstrdup(NULL) == NULL, so the ternary could be moved out the > > > xstrdup. > > > > gadget->serialnumber is freed when the value of the variable is changed, > > so it must be an allocated string. gadget->serialnumber = "unset" would > > be wrong. > > I would have done the following: > > xstrdup(barebox_get_serial_number()) ? : xstrdup("unset"); Your original code looked better. This one looks like the ?: handles failures in xstrdup(). Sascha > > > > > Sascha > > > > > > > > > dev_add_param_string(>dev, "serialnumber", NULL, NULL, > > > > >serialnumber, NULL); > > > > > > > > > > -- > > > Pengutronix e.K. | | > > > Steuerwalder Str. 21 | http://www.pengutronix.de/ | > > > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| > > > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | > > > > > > > > > > > > > -- > > Pengutronix e.K. | | > > Steuerwalder Str. 21 | http://www.pengutronix.de/ | > > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| > > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | > > > > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [PATCH v2 1/2] usb: gadget: initialize serialnumber
On 23-10-27, Sascha Hauer wrote: > On Thu, Oct 26, 2023 at 11:32:10PM +0200, Ahmad Fatoum wrote: > > On 26.10.23 16:43, Marco Felsch wrote: > > > Windows hosts do require the serial number to be set to any ascii string > > > to enumerate correctly. Set the serial number if provided or to "unset" > > > if not to provide a sane default which works for both hosts. > > > > > > Reported-by: Andrey Zhizhikin > > > Signed-off-by: Marco Felsch > > > > Reviewed-by: Ahmad Fatoum > > > > > --- > > > Changelog > > > > > > v2: > > > - adapt commit message > > > - use barebox_get_serial_number() and "unset" > > > > > > drivers/usb/gadget/udc/core.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c > > > index b58498680ad1..e7cfa0d5d836 100644 > > > --- a/drivers/usb/gadget/udc/core.c > > > +++ b/drivers/usb/gadget/udc/core.c > > > @@ -1204,7 +1204,7 @@ int usb_add_gadget(struct usb_gadget *gadget) > > > gadget->productname = xstrdup(barebox_get_model()); > > > dev_add_param_string(>dev, "productname", NULL, NULL, > > >>productname, NULL); > > > - gadget->serialnumber = xstrdup(""); > > > + gadget->serialnumber = xstrdup(barebox_get_serial_number() ? : "unset"); > > > > Nitpick: xstrdup(NULL) == NULL, so the ternary could be moved out the > > xstrdup. > > gadget->serialnumber is freed when the value of the variable is changed, > so it must be an allocated string. gadget->serialnumber = "unset" would > be wrong. I would have done the following: xstrdup(barebox_get_serial_number()) ? : xstrdup("unset"); > > Sascha > > > > > > dev_add_param_string(>dev, "serialnumber", NULL, NULL, > > >>serialnumber, NULL); > > > > > > > -- > > Pengutronix e.K. | | > > Steuerwalder Str. 21 | http://www.pengutronix.de/ | > > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| > > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | > > > > > > > > -- > Pengutronix e.K. | | > Steuerwalder Str. 21 | http://www.pengutronix.de/ | > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | >
Re: [PATCH v2 1/2] usb: gadget: initialize serialnumber
On Thu, Oct 26, 2023 at 11:32:10PM +0200, Ahmad Fatoum wrote: > On 26.10.23 16:43, Marco Felsch wrote: > > Windows hosts do require the serial number to be set to any ascii string > > to enumerate correctly. Set the serial number if provided or to "unset" > > if not to provide a sane default which works for both hosts. > > > > Reported-by: Andrey Zhizhikin > > Signed-off-by: Marco Felsch > > Reviewed-by: Ahmad Fatoum > > > --- > > Changelog > > > > v2: > > - adapt commit message > > - use barebox_get_serial_number() and "unset" > > > > drivers/usb/gadget/udc/core.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c > > index b58498680ad1..e7cfa0d5d836 100644 > > --- a/drivers/usb/gadget/udc/core.c > > +++ b/drivers/usb/gadget/udc/core.c > > @@ -1204,7 +1204,7 @@ int usb_add_gadget(struct usb_gadget *gadget) > > gadget->productname = xstrdup(barebox_get_model()); > > dev_add_param_string(>dev, "productname", NULL, NULL, > > >productname, NULL); > > - gadget->serialnumber = xstrdup(""); > > + gadget->serialnumber = xstrdup(barebox_get_serial_number() ? : "unset"); > > Nitpick: xstrdup(NULL) == NULL, so the ternary could be moved out the xstrdup. gadget->serialnumber is freed when the value of the variable is changed, so it must be an allocated string. gadget->serialnumber = "unset" would be wrong. Sascha > > > dev_add_param_string(>dev, "serialnumber", NULL, NULL, > > >serialnumber, NULL); > > > > -- > Pengutronix e.K. | | > Steuerwalder Str. 21 | http://www.pengutronix.de/ | > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | > > > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [PATCH v2 1/2] usb: gadget: initialize serialnumber
On 23-10-26, Ahmad Fatoum wrote: > On 26.10.23 16:43, Marco Felsch wrote: > > Windows hosts do require the serial number to be set to any ascii string > > to enumerate correctly. Set the serial number if provided or to "unset" > > if not to provide a sane default which works for both hosts. > > > > Reported-by: Andrey Zhizhikin > > Signed-off-by: Marco Felsch > > Reviewed-by: Ahmad Fatoum > > > --- > > Changelog > > > > v2: > > - adapt commit message > > - use barebox_get_serial_number() and "unset" > > > > drivers/usb/gadget/udc/core.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c > > index b58498680ad1..e7cfa0d5d836 100644 > > --- a/drivers/usb/gadget/udc/core.c > > +++ b/drivers/usb/gadget/udc/core.c > > @@ -1204,7 +1204,7 @@ int usb_add_gadget(struct usb_gadget *gadget) > > gadget->productname = xstrdup(barebox_get_model()); > > dev_add_param_string(>dev, "productname", NULL, NULL, > > >productname, NULL); > > - gadget->serialnumber = xstrdup(""); > > + gadget->serialnumber = xstrdup(barebox_get_serial_number() ? : "unset"); > > Nitpick: xstrdup(NULL) == NULL, so the ternary could be moved out the xstrdup. Thanks, didn't checked xstrdup(). @Sascha what's your preferred way? Regards, Marco > > > dev_add_param_string(>dev, "serialnumber", NULL, NULL, > > >serialnumber, NULL); > > > > -- > Pengutronix e.K. | | > Steuerwalder Str. 21 | http://www.pengutronix.de/ | > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | > >
Re: [PATCH v2 1/2] usb: gadget: initialize serialnumber
On 26.10.23 16:43, Marco Felsch wrote: > Windows hosts do require the serial number to be set to any ascii string > to enumerate correctly. Set the serial number if provided or to "unset" > if not to provide a sane default which works for both hosts. > > Reported-by: Andrey Zhizhikin > Signed-off-by: Marco Felsch Reviewed-by: Ahmad Fatoum > --- > Changelog > > v2: > - adapt commit message > - use barebox_get_serial_number() and "unset" > > drivers/usb/gadget/udc/core.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c > index b58498680ad1..e7cfa0d5d836 100644 > --- a/drivers/usb/gadget/udc/core.c > +++ b/drivers/usb/gadget/udc/core.c > @@ -1204,7 +1204,7 @@ int usb_add_gadget(struct usb_gadget *gadget) > gadget->productname = xstrdup(barebox_get_model()); > dev_add_param_string(>dev, "productname", NULL, NULL, >>productname, NULL); > - gadget->serialnumber = xstrdup(""); > + gadget->serialnumber = xstrdup(barebox_get_serial_number() ? : "unset"); Nitpick: xstrdup(NULL) == NULL, so the ternary could be moved out the xstrdup. > dev_add_param_string(>dev, "serialnumber", NULL, NULL, >>serialnumber, NULL); > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |