* [PATCH] usb: gadget: udc: atmel: used managed kasprintf
@ 2016-12-01 10:26 Alexandre Belloni
2016-12-01 11:20 ` Nicolas Ferre
2016-12-02 15:59 ` David Laight
0 siblings, 2 replies; 8+ messages in thread
From: Alexandre Belloni @ 2016-12-01 10:26 UTC (permalink / raw)
To: Felipe Balbi
Cc: Nicolas Ferre, linux-arm-kernel, linux-kernel, linux-usb,
Alexandre Belloni
Use devm_kasprintf instead of simple kasprintf to free the allocated memory
when needed.
Suggested-by: Peter Rosin <peda@axentia.se>
Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
drivers/usb/gadget/udc/atmel_usba_udc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
index 45bc997d0711..aec72fe8273c 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -1978,7 +1978,8 @@ static struct usba_ep * atmel_udc_of_init(struct platform_device *pdev,
dev_err(&pdev->dev, "of_probe: name error(%d)\n", ret);
goto err;
}
- ep->ep.name = kasprintf(GFP_KERNEL, "ep%d", ep->index);
+ ep->ep.name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "ep%d",
+ ep->index);
ep->ep_regs = udc->regs + USBA_EPT_BASE(i);
ep->dma_regs = udc->regs + USBA_DMA_BASE(i);
--
2.10.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] usb: gadget: udc: atmel: used managed kasprintf
2016-12-01 10:26 [PATCH] usb: gadget: udc: atmel: used managed kasprintf Alexandre Belloni
@ 2016-12-01 11:20 ` Nicolas Ferre
2016-12-02 15:59 ` David Laight
1 sibling, 0 replies; 8+ messages in thread
From: Nicolas Ferre @ 2016-12-01 11:20 UTC (permalink / raw)
To: Alexandre Belloni, Felipe Balbi; +Cc: linux-arm-kernel, linux-kernel, linux-usb
Le 01/12/2016 à 11:26, Alexandre Belloni a écrit :
> Use devm_kasprintf instead of simple kasprintf to free the allocated memory
> when needed.
>
> Suggested-by: Peter Rosin <peda@axentia.se>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> ---
> drivers/usb/gadget/udc/atmel_usba_udc.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
> index 45bc997d0711..aec72fe8273c 100644
> --- a/drivers/usb/gadget/udc/atmel_usba_udc.c
> +++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
> @@ -1978,7 +1978,8 @@ static struct usba_ep * atmel_udc_of_init(struct platform_device *pdev,
> dev_err(&pdev->dev, "of_probe: name error(%d)\n", ret);
> goto err;
> }
> - ep->ep.name = kasprintf(GFP_KERNEL, "ep%d", ep->index);
> + ep->ep.name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "ep%d",
> + ep->index);
>
> ep->ep_regs = udc->regs + USBA_EPT_BASE(i);
> ep->dma_regs = udc->regs + USBA_DMA_BASE(i);
>
--
Nicolas Ferre
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH] usb: gadget: udc: atmel: used managed kasprintf
2016-12-01 10:26 [PATCH] usb: gadget: udc: atmel: used managed kasprintf Alexandre Belloni
2016-12-01 11:20 ` Nicolas Ferre
@ 2016-12-02 15:59 ` David Laight
2016-12-02 16:19 ` Alexandre Belloni
1 sibling, 1 reply; 8+ messages in thread
From: David Laight @ 2016-12-02 15:59 UTC (permalink / raw)
To: 'Alexandre Belloni', Felipe Balbi
Cc: Nicolas Ferre, linux-arm-kernel, linux-kernel, linux-usb
From: Alexandre Belloni
> Sent: 01 December 2016 10:27
> Use devm_kasprintf instead of simple kasprintf to free the allocated memory
> when needed.
s/when needed/when the device is freed/
> Suggested-by: Peter Rosin <peda@axentia.se>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> ---
> drivers/usb/gadget/udc/atmel_usba_udc.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
> index 45bc997d0711..aec72fe8273c 100644
> --- a/drivers/usb/gadget/udc/atmel_usba_udc.c
> +++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
> @@ -1978,7 +1978,8 @@ static struct usba_ep * atmel_udc_of_init(struct platform_device *pdev,
> dev_err(&pdev->dev, "of_probe: name error(%d)\n", ret);
> goto err;
> }
> - ep->ep.name = kasprintf(GFP_KERNEL, "ep%d", ep->index);
> + ep->ep.name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "ep%d",
> + ep->index);
Acually why bother mallocing such a small string at all.
The maximum length is 12 bytes even if 'index' are unrestricted.
David
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] usb: gadget: udc: atmel: used managed kasprintf
2016-12-02 15:59 ` David Laight
@ 2016-12-02 16:19 ` Alexandre Belloni
2016-12-05 17:17 ` David Laight
0 siblings, 1 reply; 8+ messages in thread
From: Alexandre Belloni @ 2016-12-02 16:19 UTC (permalink / raw)
To: David Laight
Cc: Felipe Balbi, Nicolas Ferre, linux-arm-kernel, linux-kernel, linux-usb
On 02/12/2016 at 15:59:57 +0000, David Laight wrote :
> From: Alexandre Belloni
> > Sent: 01 December 2016 10:27
> > Use devm_kasprintf instead of simple kasprintf to free the allocated memory
> > when needed.
>
> s/when needed/when the device is freed/
>
> > Suggested-by: Peter Rosin <peda@axentia.se>
> > Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> > ---
> > drivers/usb/gadget/udc/atmel_usba_udc.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
> > index 45bc997d0711..aec72fe8273c 100644
> > --- a/drivers/usb/gadget/udc/atmel_usba_udc.c
> > +++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
> > @@ -1978,7 +1978,8 @@ static struct usba_ep * atmel_udc_of_init(struct platform_device *pdev,
> > dev_err(&pdev->dev, "of_probe: name error(%d)\n", ret);
> > goto err;
> > }
> > - ep->ep.name = kasprintf(GFP_KERNEL, "ep%d", ep->index);
> > + ep->ep.name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "ep%d",
> > + ep->index);
>
> Acually why bother mallocing such a small string at all.
> The maximum length is 12 bytes even if 'index' are unrestricted.
>
IIRC, using statically allocated string is failing somewhere is the USB
core but I don't remember all the details.
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH] usb: gadget: udc: atmel: used managed kasprintf
2016-12-02 16:19 ` Alexandre Belloni
@ 2016-12-05 17:17 ` David Laight
2017-01-16 10:27 ` Felipe Balbi
0 siblings, 1 reply; 8+ messages in thread
From: David Laight @ 2016-12-05 17:17 UTC (permalink / raw)
To: 'Alexandre Belloni'
Cc: Felipe Balbi, Nicolas Ferre, linux-arm-kernel, linux-kernel, linux-usb
From: Alexandre Belloni
> Sent: 02 December 2016 16:19
> On 02/12/2016 at 15:59:57 +0000, David Laight wrote :
> > From: Alexandre Belloni
> > > Sent: 01 December 2016 10:27
> > > Use devm_kasprintf instead of simple kasprintf to free the allocated memory
> > > when needed.
> >
> > s/when needed/when the device is freed/
> >
> > > Suggested-by: Peter Rosin <peda@axentia.se>
> > > Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> > > ---
> > > drivers/usb/gadget/udc/atmel_usba_udc.c | 3 ++-
> > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
> > > index 45bc997d0711..aec72fe8273c 100644
> > > --- a/drivers/usb/gadget/udc/atmel_usba_udc.c
> > > +++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
> > > @@ -1978,7 +1978,8 @@ static struct usba_ep * atmel_udc_of_init(struct platform_device *pdev,
> > > dev_err(&pdev->dev, "of_probe: name error(%d)\n", ret);
> > > goto err;
> > > }
> > > - ep->ep.name = kasprintf(GFP_KERNEL, "ep%d", ep->index);
> > > + ep->ep.name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "ep%d",
> > > + ep->index);
> >
> > Acually why bother mallocing such a small string at all.
> > The maximum length is 12 bytes even if 'index' are unrestricted.
> >
>
> IIRC, using statically allocated string is failing somewhere is the USB
> core but I don't remember all the details.
I can't imagine that changing ep->ep.name from 'char *' to 'char [12]' would
make any difference.
David
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH] usb: gadget: udc: atmel: used managed kasprintf
2016-12-05 17:17 ` David Laight
@ 2017-01-16 10:27 ` Felipe Balbi
2017-01-16 11:11 ` Alexandre Belloni
0 siblings, 1 reply; 8+ messages in thread
From: Felipe Balbi @ 2017-01-16 10:27 UTC (permalink / raw)
To: David Laight, 'Alexandre Belloni'
Cc: Nicolas Ferre, linux-arm-kernel, linux-kernel, linux-usb
[-- Attachment #1: Type: text/plain, Size: 1857 bytes --]
Hi,
David Laight <David.Laight@ACULAB.COM> writes:
> From: Alexandre Belloni
>> Sent: 02 December 2016 16:19
>> On 02/12/2016 at 15:59:57 +0000, David Laight wrote :
>> > From: Alexandre Belloni
>> > > Sent: 01 December 2016 10:27
>> > > Use devm_kasprintf instead of simple kasprintf to free the allocated memory
>> > > when needed.
>> >
>> > s/when needed/when the device is freed/
>> >
>> > > Suggested-by: Peter Rosin <peda@axentia.se>
>> > > Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
>> > > ---
>> > > drivers/usb/gadget/udc/atmel_usba_udc.c | 3 ++-
>> > > 1 file changed, 2 insertions(+), 1 deletion(-)
>> > >
>> > > diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
>> > > index 45bc997d0711..aec72fe8273c 100644
>> > > --- a/drivers/usb/gadget/udc/atmel_usba_udc.c
>> > > +++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
>> > > @@ -1978,7 +1978,8 @@ static struct usba_ep * atmel_udc_of_init(struct platform_device *pdev,
>> > > dev_err(&pdev->dev, "of_probe: name error(%d)\n", ret);
>> > > goto err;
>> > > }
>> > > - ep->ep.name = kasprintf(GFP_KERNEL, "ep%d", ep->index);
>> > > + ep->ep.name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "ep%d",
>> > > + ep->index);
>> >
>> > Acually why bother mallocing such a small string at all.
>> > The maximum length is 12 bytes even if 'index' are unrestricted.
>> >
>>
>> IIRC, using statically allocated string is failing somewhere is the USB
>> core but I don't remember all the details.
>
> I can't imagine that changing ep->ep.name from 'char *' to 'char [12]' would
> make any difference.
the actual name is managed by the UDC. Meaning, ep->ep.name should be a
pointer, but it could very well just point to ep->name which would be
char [12].
--
balbi
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] usb: gadget: udc: atmel: used managed kasprintf
2017-01-16 10:27 ` Felipe Balbi
@ 2017-01-16 11:11 ` Alexandre Belloni
2017-01-16 11:19 ` Felipe Balbi
0 siblings, 1 reply; 8+ messages in thread
From: Alexandre Belloni @ 2017-01-16 11:11 UTC (permalink / raw)
To: Felipe Balbi
Cc: David Laight, Nicolas Ferre, linux-arm-kernel, linux-kernel, linux-usb
On 16/01/2017 at 12:27:04 +0200, Felipe Balbi wrote :
>
> Hi,
>
> David Laight <David.Laight@ACULAB.COM> writes:
> > From: Alexandre Belloni
> >> Sent: 02 December 2016 16:19
> >> On 02/12/2016 at 15:59:57 +0000, David Laight wrote :
> >> > From: Alexandre Belloni
> >> > > Sent: 01 December 2016 10:27
> >> > > Use devm_kasprintf instead of simple kasprintf to free the allocated memory
> >> > > when needed.
> >> >
> >> > s/when needed/when the device is freed/
> >> >
> >> > > Suggested-by: Peter Rosin <peda@axentia.se>
> >> > > Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> >> > > ---
> >> > > drivers/usb/gadget/udc/atmel_usba_udc.c | 3 ++-
> >> > > 1 file changed, 2 insertions(+), 1 deletion(-)
> >> > >
> >> > > diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
> >> > > index 45bc997d0711..aec72fe8273c 100644
> >> > > --- a/drivers/usb/gadget/udc/atmel_usba_udc.c
> >> > > +++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
> >> > > @@ -1978,7 +1978,8 @@ static struct usba_ep * atmel_udc_of_init(struct platform_device *pdev,
> >> > > dev_err(&pdev->dev, "of_probe: name error(%d)\n", ret);
> >> > > goto err;
> >> > > }
> >> > > - ep->ep.name = kasprintf(GFP_KERNEL, "ep%d", ep->index);
> >> > > + ep->ep.name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "ep%d",
> >> > > + ep->index);
> >> >
> >> > Acually why bother mallocing such a small string at all.
> >> > The maximum length is 12 bytes even if 'index' are unrestricted.
> >> >
> >>
> >> IIRC, using statically allocated string is failing somewhere is the USB
> >> core but I don't remember all the details.
> >
> > I can't imagine that changing ep->ep.name from 'char *' to 'char [12]' would
> > make any difference.
>
> the actual name is managed by the UDC. Meaning, ep->ep.name should be a
> pointer, but it could very well just point to ep->name which would be
> char [12].
>
Yeah, I sent a patch that did just that.
http://lists.infradead.org/pipermail/linux-arm-kernel/2017-January/478602.html
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] usb: gadget: udc: atmel: used managed kasprintf
2017-01-16 11:11 ` Alexandre Belloni
@ 2017-01-16 11:19 ` Felipe Balbi
0 siblings, 0 replies; 8+ messages in thread
From: Felipe Balbi @ 2017-01-16 11:19 UTC (permalink / raw)
To: Alexandre Belloni
Cc: David Laight, Nicolas Ferre, linux-arm-kernel, linux-kernel, linux-usb
[-- Attachment #1: Type: text/plain, Size: 2211 bytes --]
Hi,
Alexandre Belloni <alexandre.belloni@free-electrons.com> writes:
>> David Laight <David.Laight@ACULAB.COM> writes:
>> > From: Alexandre Belloni
>> >> Sent: 02 December 2016 16:19
>> >> On 02/12/2016 at 15:59:57 +0000, David Laight wrote :
>> >> > From: Alexandre Belloni
>> >> > > Sent: 01 December 2016 10:27
>> >> > > Use devm_kasprintf instead of simple kasprintf to free the allocated memory
>> >> > > when needed.
>> >> >
>> >> > s/when needed/when the device is freed/
>> >> >
>> >> > > Suggested-by: Peter Rosin <peda@axentia.se>
>> >> > > Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
>> >> > > ---
>> >> > > drivers/usb/gadget/udc/atmel_usba_udc.c | 3 ++-
>> >> > > 1 file changed, 2 insertions(+), 1 deletion(-)
>> >> > >
>> >> > > diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
>> >> > > index 45bc997d0711..aec72fe8273c 100644
>> >> > > --- a/drivers/usb/gadget/udc/atmel_usba_udc.c
>> >> > > +++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
>> >> > > @@ -1978,7 +1978,8 @@ static struct usba_ep * atmel_udc_of_init(struct platform_device *pdev,
>> >> > > dev_err(&pdev->dev, "of_probe: name error(%d)\n", ret);
>> >> > > goto err;
>> >> > > }
>> >> > > - ep->ep.name = kasprintf(GFP_KERNEL, "ep%d", ep->index);
>> >> > > + ep->ep.name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "ep%d",
>> >> > > + ep->index);
>> >> >
>> >> > Acually why bother mallocing such a small string at all.
>> >> > The maximum length is 12 bytes even if 'index' are unrestricted.
>> >> >
>> >>
>> >> IIRC, using statically allocated string is failing somewhere is the USB
>> >> core but I don't remember all the details.
>> >
>> > I can't imagine that changing ep->ep.name from 'char *' to 'char [12]' would
>> > make any difference.
>>
>> the actual name is managed by the UDC. Meaning, ep->ep.name should be a
>> pointer, but it could very well just point to ep->name which would be
>> char [12].
>>
>
> Yeah, I sent a patch that did just that.
> http://lists.infradead.org/pipermail/linux-arm-kernel/2017-January/478602.html
it's in my fixes now :-)
--
balbi
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-01-16 11:21 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-01 10:26 [PATCH] usb: gadget: udc: atmel: used managed kasprintf Alexandre Belloni
2016-12-01 11:20 ` Nicolas Ferre
2016-12-02 15:59 ` David Laight
2016-12-02 16:19 ` Alexandre Belloni
2016-12-05 17:17 ` David Laight
2017-01-16 10:27 ` Felipe Balbi
2017-01-16 11:11 ` Alexandre Belloni
2017-01-16 11:19 ` Felipe Balbi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).