All of lore.kernel.org
 help / color / mirror / Atom feed
From: balbi@kernel.org (Felipe Balbi)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] usb: gadget: udc: atmel: used managed kasprintf
Date: Mon, 16 Jan 2017 13:19:25 +0200	[thread overview]
Message-ID: <87bmv7mev6.fsf@linux.intel.com> (raw)
In-Reply-To: <20170116111128.ft2b5aa63y2qe35p@piout.net>


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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170116/8307cc5b/attachment.sig>

WARNING: multiple messages have this Message-ID (diff)
From: Felipe Balbi <balbi@kernel.org>
To: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Cc: David Laight <David.Laight@ACULAB.COM>,
	Nicolas Ferre <nicolas.ferre@atmel.com>,
	"linux-arm-kernel\@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"linux-kernel\@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-usb\@vger.kernel.org" <linux-usb@vger.kernel.org>
Subject: Re: [PATCH] usb: gadget: udc: atmel: used managed kasprintf
Date: Mon, 16 Jan 2017 13:19:25 +0200	[thread overview]
Message-ID: <87bmv7mev6.fsf@linux.intel.com> (raw)
In-Reply-To: <20170116111128.ft2b5aa63y2qe35p@piout.net>

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

  reply	other threads:[~2017-01-16 11:19 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-01 10:26 [PATCH] usb: gadget: udc: atmel: used managed kasprintf Alexandre Belloni
2016-12-01 10:26 ` Alexandre Belloni
2016-12-01 11:20 ` Nicolas Ferre
2016-12-01 11:20   ` Nicolas Ferre
2016-12-02 15:59 ` David Laight
2016-12-02 15:59   ` David Laight
2016-12-02 16:19   ` Alexandre Belloni
2016-12-02 16:19     ` Alexandre Belloni
2016-12-05 17:17     ` David Laight
2016-12-05 17:17       ` David Laight
2017-01-16 10:27       ` Felipe Balbi
2017-01-16 10:27         ` Felipe Balbi
2017-01-16 11:11         ` Alexandre Belloni
2017-01-16 11:11           ` Alexandre Belloni
2017-01-16 11:19           ` Felipe Balbi [this message]
2017-01-16 11:19             ` Felipe Balbi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87bmv7mev6.fsf@linux.intel.com \
    --to=balbi@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.