From: Jonathan Cameron <jic23@kernel.org>
To: Lars-Peter Clausen <lars@metafoo.de>
Cc: linux-iio@vger.kernel.org
Subject: Re: [PATCH 2/2] iio: Avoid unnecessary kasprintf
Date: Tue, 18 Feb 2014 10:31:55 +0000 [thread overview]
Message-ID: <5303369B.2030301@kernel.org> (raw)
In-Reply-To: <1392387566-8411-2-git-send-email-lars@metafoo.de>
On 14/02/14 14:19, Lars-Peter Clausen wrote:
> name_format already contains the final name and no format characters. So the
> code basically reads:
>
> dev_attr->attr.name = kstrdup(GFP_KERNEL, name_format);
> if (dev_attr->attr.name == NULL)
> ...
> kfree(name_format);
>
> Which means we can save one alloc and free pair per attribute name if we
> directly assign name_format to dev_attr->attr.name.
>
> The patch also renames name_format to name to denote that this is indeed the
> final name and has no format characters in it.
>
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---
> I went back through the log and it looks like it has always been like this.
> name_format never actually contained a format string.
oops ;)
It did in my mind at some point.
Good spot. Applied to the togreg branch of iio.git.
> ---
> drivers/iio/industrialio-core.c | 39 +++++++++++++--------------------------
> 1 file changed, 13 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index fd96e8b..85a2a07 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -540,7 +540,7 @@ int __iio_device_attr_init(struct device_attribute *dev_attr,
> enum iio_shared_by shared_by)
> {
> int ret = 0;
> - char *name_format = NULL;
> + char *name = NULL;
> char *full_postfix;
> sysfs_attr_init(&dev_attr->attr);
>
> @@ -572,16 +572,15 @@ int __iio_device_attr_init(struct device_attribute *dev_attr,
> if (chan->differential) { /* Differential can not have modifier */
> switch (shared_by) {
> case IIO_SHARED_BY_ALL:
> - name_format = kasprintf(GFP_KERNEL, "%s", full_postfix);
> + name = kasprintf(GFP_KERNEL, "%s", full_postfix);
> break;
> case IIO_SHARED_BY_DIR:
> - name_format = kasprintf(GFP_KERNEL, "%s_%s",
> + name = kasprintf(GFP_KERNEL, "%s_%s",
> iio_direction[chan->output],
> full_postfix);
> break;
> case IIO_SHARED_BY_TYPE:
> - name_format
> - = kasprintf(GFP_KERNEL, "%s_%s-%s_%s",
> + name = kasprintf(GFP_KERNEL, "%s_%s-%s_%s",
> iio_direction[chan->output],
> iio_chan_type_name_spec[chan->type],
> iio_chan_type_name_spec[chan->type],
> @@ -593,8 +592,7 @@ int __iio_device_attr_init(struct device_attribute *dev_attr,
> ret = -EINVAL;
> goto error_free_full_postfix;
> }
> - name_format
> - = kasprintf(GFP_KERNEL,
> + name = kasprintf(GFP_KERNEL,
> "%s_%s%d-%s%d_%s",
> iio_direction[chan->output],
> iio_chan_type_name_spec[chan->type],
> @@ -607,16 +605,15 @@ int __iio_device_attr_init(struct device_attribute *dev_attr,
> } else { /* Single ended */
> switch (shared_by) {
> case IIO_SHARED_BY_ALL:
> - name_format = kasprintf(GFP_KERNEL, "%s", full_postfix);
> + name = kasprintf(GFP_KERNEL, "%s", full_postfix);
> break;
> case IIO_SHARED_BY_DIR:
> - name_format = kasprintf(GFP_KERNEL, "%s_%s",
> + name = kasprintf(GFP_KERNEL, "%s_%s",
> iio_direction[chan->output],
> full_postfix);
> break;
> case IIO_SHARED_BY_TYPE:
> - name_format
> - = kasprintf(GFP_KERNEL, "%s_%s_%s",
> + name = kasprintf(GFP_KERNEL, "%s_%s_%s",
> iio_direction[chan->output],
> iio_chan_type_name_spec[chan->type],
> full_postfix);
> @@ -624,33 +621,24 @@ int __iio_device_attr_init(struct device_attribute *dev_attr,
>
> case IIO_SEPARATE:
> if (chan->indexed)
> - name_format
> - = kasprintf(GFP_KERNEL, "%s_%s%d_%s",
> + name = kasprintf(GFP_KERNEL, "%s_%s%d_%s",
> iio_direction[chan->output],
> iio_chan_type_name_spec[chan->type],
> chan->channel,
> full_postfix);
> else
> - name_format
> - = kasprintf(GFP_KERNEL, "%s_%s_%s",
> + name = kasprintf(GFP_KERNEL, "%s_%s_%s",
> iio_direction[chan->output],
> iio_chan_type_name_spec[chan->type],
> full_postfix);
> break;
> }
> }
> - if (name_format == NULL) {
> + if (name == NULL) {
> ret = -ENOMEM;
> goto error_free_full_postfix;
> }
> - dev_attr->attr.name = kasprintf(GFP_KERNEL,
> - name_format,
> - chan->channel,
> - chan->channel2);
> - if (dev_attr->attr.name == NULL) {
> - ret = -ENOMEM;
> - goto error_free_name_format;
> - }
> + dev_attr->attr.name = name;
>
> if (readfunc) {
> dev_attr->attr.mode |= S_IRUGO;
> @@ -661,8 +649,7 @@ int __iio_device_attr_init(struct device_attribute *dev_attr,
> dev_attr->attr.mode |= S_IWUSR;
> dev_attr->store = writefunc;
> }
> -error_free_name_format:
> - kfree(name_format);
> +
> error_free_full_postfix:
> kfree(full_postfix);
>
>
next prev parent reply other threads:[~2014-02-18 10:31 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-02-14 14:19 [PATCH 1/2] iio: Don't include extended name in shared attributes Lars-Peter Clausen
2014-02-14 14:19 ` [PATCH 2/2] iio: Avoid unnecessary kasprintf Lars-Peter Clausen
2014-02-18 10:31 ` Jonathan Cameron [this message]
2014-02-18 10:27 ` [PATCH 1/2] iio: Don't include extended name in shared attributes Jonathan Cameron
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=5303369B.2030301@kernel.org \
--to=jic23@kernel.org \
--cc=lars@metafoo.de \
--cc=linux-iio@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;
as well as URLs for NNTP newsgroup(s).