2016-12-01 10:27:03

by Alexandre Belloni

[permalink] [raw]
Subject: [PATCH] usb: gadget: udc: atmel: used managed kasprintf

Use devm_kasprintf instead of simple kasprintf to free the allocated memory
when needed.

Suggested-by: Peter Rosin <[email protected]>
Signed-off-by: Alexandre Belloni <[email protected]>
---
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


2016-12-01 11:20:18

by Nicolas Ferre

[permalink] [raw]
Subject: Re: [PATCH] usb: gadget: udc: atmel: used managed kasprintf

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 <[email protected]>
> Signed-off-by: Alexandre Belloni <[email protected]>

Acked-by: Nicolas Ferre <[email protected]>

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

2016-12-02 16:00:08

by David Laight

[permalink] [raw]
Subject: RE: [PATCH] usb: gadget: udc: atmel: used managed kasprintf

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 <[email protected]>
> Signed-off-by: Alexandre Belloni <[email protected]>
> ---
> 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

2016-12-02 16:20:58

by Alexandre Belloni

[permalink] [raw]
Subject: Re: [PATCH] usb: gadget: udc: atmel: used managed kasprintf

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 <[email protected]>
> > Signed-off-by: Alexandre Belloni <[email protected]>
> > ---
> > 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

2016-12-05 17:17:14

by David Laight

[permalink] [raw]
Subject: RE: [PATCH] usb: gadget: udc: atmel: used managed kasprintf

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 <[email protected]>
> > > Signed-off-by: Alexandre Belloni <[email protected]>
> > > ---
> > > 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