* [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 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.