public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Darren Hart <dvhart@infradead.org>
To: David Laight <David.Laight@ACULAB.COM>
Cc: "'Andy Shevchenko'" <andriy.shevchenko@linux.intel.com>,
	"platform-driver-x86@vger.kernel.org"
	<platform-driver-x86@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v1] platform/x86: wmi: Replace kmalloc + sprintf() with kasprintf()
Date: Fri, 16 Feb 2018 11:54:10 -0800	[thread overview]
Message-ID: <20180216195410.GF5786@fury> (raw)
In-Reply-To: <829ccf8330b7418a976205665e0ab4bd@AcuMS.aculab.com>

On Fri, Feb 16, 2018 at 03:55:24PM +0000, David Laight wrote:
> From: Andy Shevchenko
> > Sent: 16 February 2018 15:40
> >
> > kasprintf() does the job of two: kmalloc() and sprintf().
> > Replace two calls with one.
> ...
> > -		buf = kmalloc(strlen(wdriver->driver.name) + 5, GFP_KERNEL);
> > +		buf = kasprintf(GFP_KERNEL, "wmi/%s", wdriver->driver.name);
> ...
> 
> Except that kasprintf() has no idea how long a buffer is needed.
> It might even do the printf twice just to get the length.

Sure, but this is one-time and non-critical path. Unfortunately there
isn't any guidance in Documentation here. Of the 520 or so instances of
kasprintf usages in the kernel, device name or similar is a very common
pattern. Eliminating manual counting of characters for buffer allocation
seems like a good plan to me.

So unless we want to argue that all those use cases are wrong, this
would appear to be at least common practice, if not best practice for
this particular pattern.

Reviewed-by: Darren Hart (VMware) <dvhart@infradead.org>

-- 
Darren Hart
VMware Open Source Technology Center

      parent reply	other threads:[~2018-02-16 19:54 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-16 15:40 [PATCH v1] platform/x86: wmi: Replace kmalloc + sprintf() with kasprintf() Andy Shevchenko
2018-02-16 15:55 ` David Laight
2018-02-16 19:30   ` Andy Shevchenko
2018-02-16 19:54   ` Darren Hart [this message]

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=20180216195410.GF5786@fury \
    --to=dvhart@infradead.org \
    --cc=David.Laight@ACULAB.COM \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=platform-driver-x86@vger.kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox