| Message ID | 20171207094209.14748-1-lars@metafoo.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 7 Dec 2017 10:42:09 +0100 Lars-Peter Clausen <lars@metafoo.de> wrote: > Some enums might have gaps or reserved values in the middle of their value > range. E.g. consider a 2-bit enum where the values 0, 1 and 3 have a > meaning, but 2 is a reserved value and must not be used. > > Add support for such enums to the IIO enum helper functions. A reserved > value is marked by setting its entry in the items array to NULL rather > than the normal descriptive string value. > > Unfortunately that means we can no longer use __sysfs_match_string() since > it treats a NULL entry as the array terminator. But the amount of > boilerplate is manageable. > > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> > --- > Open for suggestions on how to massage __sysfs_match_string() into handling > this use-case. In theory - that function should only use the null terminator in the case of n == -1. Or so it's documentation implies... However to make that actually the case, we would have to be sure of causing no breakage which would be hard to do. Mind you it's only used in a few places, so you could check them. From a quick scan over them it looks like you would fine to make the change (the single non IIO user is clearly fine.) > --- > drivers/iio/industrialio-core.c | 21 +++++++++++++++------ > 1 file changed, 15 insertions(+), 6 deletions(-) > > diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c > index 2e8e36f9da61..e69bef13d005 100644 > --- a/drivers/iio/industrialio-core.c > +++ b/drivers/iio/industrialio-core.c > @@ -446,8 +446,12 @@ ssize_t iio_enum_available_read(struct iio_dev *indio_dev, > if (!e->num_items) > return 0; > > - for (i = 0; i < e->num_items; ++i) > + for (i = 0; i < e->num_items; ++i) { > + if (!e->items[i]) > + continue; > + > len += scnprintf(buf + len, PAGE_SIZE - len, "%s ", e->items[i]); > + } > > /* replace last space with a newline */ > buf[len - 1] = '\n'; > @@ -468,7 +472,7 @@ ssize_t iio_enum_read(struct iio_dev *indio_dev, > i = e->get(indio_dev, chan); > if (i < 0) > return i; > - else if (i >= e->num_items) > + else if (i >= e->num_items || !e->items[i]) > return -EINVAL; > > return snprintf(buf, PAGE_SIZE, "%s\n", e->items[i]); > @@ -480,16 +484,21 @@ ssize_t iio_enum_write(struct iio_dev *indio_dev, > size_t len) > { > const struct iio_enum *e = (const struct iio_enum *)priv; > + unsigned int i; > int ret; > > if (!e->set) > return -EINVAL; > > - ret = __sysfs_match_string(e->items, e->num_items, buf); > - if (ret < 0) > - return ret; > + for (i = 0; i < e->num_items; i++) { > + if (e->items[i] && sysfs_streq(buf, e->items[i])) > + break; > + } > + > + if (i == e->num_items) > + return -EINVAL; > > - ret = e->set(indio_dev, chan, ret); > + ret = e->set(indio_dev, chan, i); > return ret ? ret : len; > } > EXPORT_SYMBOL_GPL(iio_enum_write); -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 2017-12-07 at 10:42 +0100, Lars-Peter Clausen wrote: > Open for suggestions on how to massage __sysfs_match_string() into > handling > this use-case. I use to have an idea to use bitmap variant of such functions. Though I didn't find enough users to make it beneficial. match_string_[with_gaps](...unsigned long *map, ...) { ... for_each_set_bit() { ... } ... } So, in addition to (or instead of) passing size pass bitmap.
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c index 2e8e36f9da61..e69bef13d005 100644 --- a/drivers/iio/industrialio-core.c +++ b/drivers/iio/industrialio-core.c @@ -446,8 +446,12 @@ ssize_t iio_enum_available_read(struct iio_dev *indio_dev, if (!e->num_items) return 0; - for (i = 0; i < e->num_items; ++i) + for (i = 0; i < e->num_items; ++i) { + if (!e->items[i]) + continue; + len += scnprintf(buf + len, PAGE_SIZE - len, "%s ", e->items[i]); + } /* replace last space with a newline */ buf[len - 1] = '\n'; @@ -468,7 +472,7 @@ ssize_t iio_enum_read(struct iio_dev *indio_dev, i = e->get(indio_dev, chan); if (i < 0) return i; - else if (i >= e->num_items) + else if (i >= e->num_items || !e->items[i]) return -EINVAL; return snprintf(buf, PAGE_SIZE, "%s\n", e->items[i]); @@ -480,16 +484,21 @@ ssize_t iio_enum_write(struct iio_dev *indio_dev, size_t len) { const struct iio_enum *e = (const struct iio_enum *)priv; + unsigned int i; int ret; if (!e->set) return -EINVAL; - ret = __sysfs_match_string(e->items, e->num_items, buf); - if (ret < 0) - return ret; + for (i = 0; i < e->num_items; i++) { + if (e->items[i] && sysfs_streq(buf, e->items[i])) + break; + } + + if (i == e->num_items) + return -EINVAL; - ret = e->set(indio_dev, chan, ret); + ret = e->set(indio_dev, chan, i); return ret ? ret : len; } EXPORT_SYMBOL_GPL(iio_enum_write);
Some enums might have gaps or reserved values in the middle of their value range. E.g. consider a 2-bit enum where the values 0, 1 and 3 have a meaning, but 2 is a reserved value and must not be used. Add support for such enums to the IIO enum helper functions. A reserved value is marked by setting its entry in the items array to NULL rather than the normal descriptive string value. Unfortunately that means we can no longer use __sysfs_match_string() since it treats a NULL entry as the array terminator. But the amount of boilerplate is manageable. Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> --- Open for suggestions on how to massage __sysfs_match_string() into handling this use-case. --- drivers/iio/industrialio-core.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-)