mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH v2 1/2] usb: gadget: initialize serialnumber
@ 2023-10-26 14:43 Marco Felsch
  2023-10-26 14:43 ` [PATCH v2 2/2] scripts: get_maintainers: drop --status enforcement Marco Felsch
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Marco Felsch @ 2023-10-26 14:43 UTC (permalink / raw)
  To: barebox

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 <andrey.zhizhikin@leica-geosystems.com>
Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
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(&gadget->dev, "productname", NULL, NULL,
 			     &gadget->productname, NULL);
-	gadget->serialnumber = xstrdup("");
+	gadget->serialnumber = xstrdup(barebox_get_serial_number() ? : "unset");
 	dev_add_param_string(&gadget->dev, "serialnumber", NULL, NULL,
 			     &gadget->serialnumber, NULL);
 
-- 
2.39.2




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

* [PATCH v2 2/2] scripts: get_maintainers: drop --status enforcement
  2023-10-26 14:43 [PATCH v2 1/2] usb: gadget: initialize serialnumber Marco Felsch
@ 2023-10-26 14:43 ` Marco Felsch
  2023-10-26 21:31   ` Ahmad Fatoum
  2023-10-26 21:32 ` [PATCH v2 1/2] usb: gadget: initialize serialnumber Ahmad Fatoum
  2023-11-01  8:50 ` Sascha Hauer
  2 siblings, 1 reply; 11+ messages in thread
From: Marco Felsch @ 2023-10-26 14:43 UTC (permalink / raw)
  To: barebox

The status parameter is never used so drop it and stop bother the user
of the script.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 scripts/get_maintainer.pl | 2 --
 1 file changed, 2 deletions(-)

diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index 7c0d50333400..a19c0e7305d0 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -2,8 +2,6 @@
 # SPDX-License-Identifier: GPL-2.0-only
 # Dummy get_maintainer.pl script for checkpatch.pl to use
 
-die "USAGE: get_maintainer.pl --status\n" unless grep /--status/, @ARGV;
-
 print <<'EOT'
 Sascha Hauer <s.hauer@pengutronix.de> (maintainer:BAREBOX)
 barebox@lists.infradead.org (open list:BAREBOX)
-- 
2.39.2




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

* Re: [PATCH v2 2/2] scripts: get_maintainers: drop --status enforcement
  2023-10-26 14:43 ` [PATCH v2 2/2] scripts: get_maintainers: drop --status enforcement Marco Felsch
@ 2023-10-26 21:31   ` Ahmad Fatoum
  0 siblings, 0 replies; 11+ messages in thread
From: Ahmad Fatoum @ 2023-10-26 21:31 UTC (permalink / raw)
  To: Marco Felsch, barebox

On 26.10.23 16:43, Marco Felsch wrote:
> The status parameter is never used so drop it and stop bother the user
> of the script.
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>

Acked-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

> ---
>  scripts/get_maintainer.pl | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
> index 7c0d50333400..a19c0e7305d0 100755
> --- a/scripts/get_maintainer.pl
> +++ b/scripts/get_maintainer.pl
> @@ -2,8 +2,6 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  # Dummy get_maintainer.pl script for checkpatch.pl to use
>  
> -die "USAGE: get_maintainer.pl --status\n" unless grep /--status/, @ARGV;
> -
>  print <<'EOT'
>  Sascha Hauer <s.hauer@pengutronix.de> (maintainer:BAREBOX)
>  barebox@lists.infradead.org (open list:BAREBOX)

-- 
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-5555 |




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

* Re: [PATCH v2 1/2] usb: gadget: initialize serialnumber
  2023-10-26 14:43 [PATCH v2 1/2] usb: gadget: initialize serialnumber Marco Felsch
  2023-10-26 14:43 ` [PATCH v2 2/2] scripts: get_maintainers: drop --status enforcement Marco Felsch
@ 2023-10-26 21:32 ` Ahmad Fatoum
  2023-10-27  5:28   ` Marco Felsch
  2023-10-27  8:00   ` Sascha Hauer
  2023-11-01  8:50 ` Sascha Hauer
  2 siblings, 2 replies; 11+ messages in thread
From: Ahmad Fatoum @ 2023-10-26 21:32 UTC (permalink / raw)
  To: Marco Felsch, barebox

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 <andrey.zhizhikin@leica-geosystems.com>
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

> ---
> 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(&gadget->dev, "productname", NULL, NULL,
>  			     &gadget->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(&gadget->dev, "serialnumber", NULL, NULL,
>  			     &gadget->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-5555 |




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

* Re: [PATCH v2 1/2] usb: gadget: initialize serialnumber
  2023-10-26 21:32 ` [PATCH v2 1/2] usb: gadget: initialize serialnumber Ahmad Fatoum
@ 2023-10-27  5:28   ` Marco Felsch
  2023-10-27  8:00   ` Sascha Hauer
  1 sibling, 0 replies; 11+ messages in thread
From: Marco Felsch @ 2023-10-27  5:28 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox

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 <andrey.zhizhikin@leica-geosystems.com>
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> 
> Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> 
> > ---
> > 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(&gadget->dev, "productname", NULL, NULL,
> >  			     &gadget->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(&gadget->dev, "serialnumber", NULL, NULL,
> >  			     &gadget->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-5555 |
> 
> 



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

* Re: [PATCH v2 1/2] usb: gadget: initialize serialnumber
  2023-10-26 21:32 ` [PATCH v2 1/2] usb: gadget: initialize serialnumber Ahmad Fatoum
  2023-10-27  5:28   ` Marco Felsch
@ 2023-10-27  8:00   ` Sascha Hauer
  2023-10-27  8:09     ` Marco Felsch
  1 sibling, 1 reply; 11+ messages in thread
From: Sascha Hauer @ 2023-10-27  8:00 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: Marco Felsch, barebox

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 <andrey.zhizhikin@leica-geosystems.com>
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> 
> Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> 
> > ---
> > 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(&gadget->dev, "productname", NULL, NULL,
> >  			     &gadget->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(&gadget->dev, "serialnumber", NULL, NULL,
> >  			     &gadget->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-5555 |
> 
> 
> 

-- 
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-5555 |



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

* Re: [PATCH v2 1/2] usb: gadget: initialize serialnumber
  2023-10-27  8:00   ` Sascha Hauer
@ 2023-10-27  8:09     ` Marco Felsch
  2023-10-27  8:17       ` Sascha Hauer
  0 siblings, 1 reply; 11+ messages in thread
From: Marco Felsch @ 2023-10-27  8:09 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: Ahmad Fatoum, barebox

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 <andrey.zhizhikin@leica-geosystems.com>
> > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > 
> > Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> > 
> > > ---
> > > 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(&gadget->dev, "productname", NULL, NULL,
> > >  			     &gadget->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(&gadget->dev, "serialnumber", NULL, NULL,
> > >  			     &gadget->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-5555 |
> > 
> > 
> > 
> 
> -- 
> 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-5555 |
> 



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

* Re: [PATCH v2 1/2] usb: gadget: initialize serialnumber
  2023-10-27  8:09     ` Marco Felsch
@ 2023-10-27  8:17       ` Sascha Hauer
  2023-10-27  8:19         ` Marco Felsch
  0 siblings, 1 reply; 11+ messages in thread
From: Sascha Hauer @ 2023-10-27  8:17 UTC (permalink / raw)
  To: Marco Felsch; +Cc: Ahmad Fatoum, barebox

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 <andrey.zhizhikin@leica-geosystems.com>
> > > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > > 
> > > Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> > > 
> > > > ---
> > > > 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(&gadget->dev, "productname", NULL, NULL,
> > > >  			     &gadget->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(&gadget->dev, "serialnumber", NULL, NULL,
> > > >  			     &gadget->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-5555 |
> > > 
> > > 
> > > 
> > 
> > -- 
> > 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-5555 |
> > 
> 
> 

-- 
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-5555 |



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

* Re: [PATCH v2 1/2] usb: gadget: initialize serialnumber
  2023-10-27  8:17       ` Sascha Hauer
@ 2023-10-27  8:19         ` Marco Felsch
  2023-11-01  7:24           ` Ahmad Fatoum
  0 siblings, 1 reply; 11+ messages in thread
From: Marco Felsch @ 2023-10-27  8:19 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: Ahmad Fatoum, barebox

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 <andrey.zhizhikin@leica-geosystems.com>
> > > > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > > > 
> > > > Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> > > > 
> > > > > ---
> > > > > 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(&gadget->dev, "productname", NULL, NULL,
> > > > >  			     &gadget->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



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

* Re: [PATCH v2 1/2] usb: gadget: initialize serialnumber
  2023-10-27  8:19         ` Marco Felsch
@ 2023-11-01  7:24           ` Ahmad Fatoum
  0 siblings, 0 replies; 11+ messages in thread
From: Ahmad Fatoum @ 2023-11-01  7:24 UTC (permalink / raw)
  To: Marco Felsch, Sascha Hauer; +Cc: barebox

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 <andrey.zhizhikin@leica-geosystems.com>
>>>>>> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
>>>>>
>>>>> Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>>>>>
>>>>>> ---
>>>>>> 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(&gadget->dev, "productname", NULL, NULL,
>>>>>>  			     &gadget->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-5555 |




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

* Re: [PATCH v2 1/2] usb: gadget: initialize serialnumber
  2023-10-26 14:43 [PATCH v2 1/2] usb: gadget: initialize serialnumber Marco Felsch
  2023-10-26 14:43 ` [PATCH v2 2/2] scripts: get_maintainers: drop --status enforcement Marco Felsch
  2023-10-26 21:32 ` [PATCH v2 1/2] usb: gadget: initialize serialnumber Ahmad Fatoum
@ 2023-11-01  8:50 ` Sascha Hauer
  2 siblings, 0 replies; 11+ messages in thread
From: Sascha Hauer @ 2023-11-01  8:50 UTC (permalink / raw)
  To: Marco Felsch; +Cc: barebox

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 <andrey.zhizhikin@leica-geosystems.com>
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
> 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(&gadget->dev, "productname", NULL, NULL,
>  			     &gadget->productname, NULL);
> -	gadget->serialnumber = xstrdup("");
> +	gadget->serialnumber = xstrdup(barebox_get_serial_number() ? : "unset");
>  	dev_add_param_string(&gadget->dev, "serialnumber", NULL, NULL,
>  			     &gadget->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-5555 |



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

end of thread, other threads:[~2023-11-01  8:52 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-26 14:43 [PATCH v2 1/2] usb: gadget: initialize serialnumber Marco Felsch
2023-10-26 14:43 ` [PATCH v2 2/2] scripts: get_maintainers: drop --status enforcement Marco Felsch
2023-10-26 21:31   ` Ahmad Fatoum
2023-10-26 21:32 ` [PATCH v2 1/2] usb: gadget: initialize serialnumber Ahmad Fatoum
2023-10-27  5:28   ` Marco Felsch
2023-10-27  8:00   ` Sascha Hauer
2023-10-27  8:09     ` Marco Felsch
2023-10-27  8:17       ` Sascha Hauer
2023-10-27  8:19         ` Marco Felsch
2023-11-01  7:24           ` Ahmad Fatoum
2023-11-01  8:50 ` Sascha Hauer

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