* [PATCH 1/2] iio: Don't include extended name in shared attributes @ 2014-02-14 14:19 Lars-Peter Clausen 2014-02-14 14:19 ` [PATCH 2/2] iio: Avoid unnecessary kasprintf Lars-Peter Clausen 2014-02-18 10:27 ` [PATCH 1/2] iio: Don't include extended name in shared attributes Jonathan Cameron 0 siblings, 2 replies; 4+ messages in thread From: Lars-Peter Clausen @ 2014-02-14 14:19 UTC (permalink / raw) To: Jonathan Cameron; +Cc: linux-iio, Lars-Peter Clausen The extended name is channel specific and should not be included in shared attributes. Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> --- It's a fix, but there are no drivers yet, which have channels with extended names and shared attributes, so no need to apply it to the fixes branch. --- drivers/iio/industrialio-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c index acc911a..fd96e8b 100644 --- a/drivers/iio/industrialio-core.c +++ b/drivers/iio/industrialio-core.c @@ -558,7 +558,7 @@ int __iio_device_attr_init(struct device_attribute *dev_attr, ->channel2], postfix); } else { - if (chan->extend_name == NULL) + if (chan->extend_name == NULL || shared_by != IIO_SEPARATE) full_postfix = kstrdup(postfix, GFP_KERNEL); else full_postfix = kasprintf(GFP_KERNEL, -- 1.8.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] iio: Avoid unnecessary kasprintf 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 ` Lars-Peter Clausen 2014-02-18 10:31 ` Jonathan Cameron 2014-02-18 10:27 ` [PATCH 1/2] iio: Don't include extended name in shared attributes Jonathan Cameron 1 sibling, 1 reply; 4+ messages in thread From: Lars-Peter Clausen @ 2014-02-14 14:19 UTC (permalink / raw) To: Jonathan Cameron; +Cc: linux-iio, Lars-Peter Clausen 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. --- 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); -- 1.8.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] iio: Avoid unnecessary kasprintf 2014-02-14 14:19 ` [PATCH 2/2] iio: Avoid unnecessary kasprintf Lars-Peter Clausen @ 2014-02-18 10:31 ` Jonathan Cameron 0 siblings, 0 replies; 4+ messages in thread From: Jonathan Cameron @ 2014-02-18 10:31 UTC (permalink / raw) To: Lars-Peter Clausen; +Cc: linux-iio 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); > > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] iio: Don't include extended name in shared attributes 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:27 ` Jonathan Cameron 1 sibling, 0 replies; 4+ messages in thread From: Jonathan Cameron @ 2014-02-18 10:27 UTC (permalink / raw) To: Lars-Peter Clausen; +Cc: linux-iio On 14/02/14 14:19, Lars-Peter Clausen wrote: > The extended name is channel specific and should not be included in shared > attributes. > > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> Applied to the togreg branch of iio.git > --- > It's a fix, but there are no drivers yet, which have channels with extended > names and shared attributes, so no need to apply it to the fixes branch. Thanks for making this clear - saves me checking ;) > --- > drivers/iio/industrialio-core.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c > index acc911a..fd96e8b 100644 > --- a/drivers/iio/industrialio-core.c > +++ b/drivers/iio/industrialio-core.c > @@ -558,7 +558,7 @@ int __iio_device_attr_init(struct device_attribute *dev_attr, > ->channel2], > postfix); > } else { > - if (chan->extend_name == NULL) > + if (chan->extend_name == NULL || shared_by != IIO_SEPARATE) > full_postfix = kstrdup(postfix, GFP_KERNEL); > else > full_postfix = kasprintf(GFP_KERNEL, > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-02-18 10:31 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 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 2014-02-18 10:27 ` [PATCH 1/2] iio: Don't include extended name in shared attributes Jonathan Cameron
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).