linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 1/2] lib/string.c: add __sysfs_match_string_with_gaps() helper
@ 2020-12-22 13:13 Alexandru Ardelean
  2020-12-22 13:13 ` [PATCH v6 2/2] iio: Handle enumerated properties with gaps Alexandru Ardelean
  2020-12-22 13:42 ` [PATCH v6 1/2] lib/string.c: add __sysfs_match_string_with_gaps() helper Andy Shevchenko
  0 siblings, 2 replies; 4+ messages in thread
From: Alexandru Ardelean @ 2020-12-22 13:13 UTC (permalink / raw)
  To: linux-iio, linux-kernel
  Cc: jic23, lars, akpm, gregkh, andy.shevchenko, Alexandru Ardelean

The original docstring of the __sysfs_match_string() and match_string()
helper, implied that -1 could be used to search through NULL terminated
arrays, and positive 'n' could be used to go through arrays that may have
NULL elements in the middle of the array.

This isn't true. Regardless of the value of 'n', the first NULL element in
the array will stop the search, even if the element may be after a NULL
element.

To allow for a behavior where we can use the __sysfs_match_string() to
search over arrays with NULL elements in the middle, the
__sysfs_match_string_with_gaps() helper is added.
If n > 0, the search will continue until the element is found or n is
reached.
If n < 0, the search will continue until the element is found or a NULL
character is found.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---

Changelog v5 -> v6
* https://lore.kernel.org/linux-iio/20201222095210.61897-1-alexandru.ardelean@analog.com/
* Fixed/updated comment; one sentence was stopping midway

 include/linux/string.h |  1 +
 lib/string.c           | 55 +++++++++++++++++++++++++++++++++---------
 2 files changed, 44 insertions(+), 12 deletions(-)

diff --git a/include/linux/string.h b/include/linux/string.h
index b1f3894a0a3e..d7999e50dae1 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -191,6 +191,7 @@ static inline int strtobool(const char *s, bool *res)
 
 int match_string(const char * const *array, size_t n, const char *string);
 int __sysfs_match_string(const char * const *array, size_t n, const char *s);
+int __sysfs_match_string_with_gaps(const char * const *array, ssize_t n, const char *s);
 
 /**
  * sysfs_match_string - matches given string in an array
diff --git a/lib/string.c b/lib/string.c
index 4288e0158d47..3dbd362f28a2 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -751,6 +751,26 @@ int match_string(const char * const *array, size_t n, const char *string)
 }
 EXPORT_SYMBOL(match_string);
 
+static int __sysfs_match_string_common(const char * const *array, ssize_t n,
+				       const char *str, bool gaps)
+{
+	const char *item;
+	int index;
+
+	for (index = 0; index < n; index++) {
+		item = array[index];
+		if (!item) {
+			if (gaps && n > 0)
+				continue;
+			break;
+		}
+		if (sysfs_streq(item, str))
+			return index;
+	}
+
+	return -EINVAL;
+}
+
 /**
  * __sysfs_match_string - matches given string in an array
  * @array: array of strings
@@ -770,21 +790,32 @@ EXPORT_SYMBOL(match_string);
  */
 int __sysfs_match_string(const char * const *array, size_t n, const char *str)
 {
-	const char *item;
-	int index;
-
-	for (index = 0; index < n; index++) {
-		item = array[index];
-		if (!item)
-			break;
-		if (sysfs_streq(item, str))
-			return index;
-	}
-
-	return -EINVAL;
+	return __sysfs_match_string_common(array, n, str, false);
 }
 EXPORT_SYMBOL(__sysfs_match_string);
 
+/**
+ * __sysfs_match_string_with_gaps - matches given string in an array with gaps
+ * @array: array of strings
+ * @n: number of strings in the array or negative for NULL terminated arrays
+ * @str: string to match with
+ *
+ * Returns index of @str in the @array or -EINVAL, similar to match_string().
+ * Uses sysfs_streq instead of strcmp for matching.
+ *
+ * This routine will look for a string in an array of strings.
+ * If n > 0, then the search will continue until the element is found or
+ * the n-th element is reached, regardless of any NULL elements in the array.
+ * If n < 0, then the search will continue until the element is found or
+ * util the first NULL element.
+ */
+int __sysfs_match_string_with_gaps(const char * const *array, ssize_t n,
+				   const char *str)
+{
+	return __sysfs_match_string_common(array, n, str, true);
+}
+EXPORT_SYMBOL(__sysfs_match_string_with_gaps);
+
 #ifndef __HAVE_ARCH_MEMSET
 /**
  * memset - Fill a region of memory with the given value
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v6 2/2] iio: Handle enumerated properties with gaps
  2020-12-22 13:13 [PATCH v6 1/2] lib/string.c: add __sysfs_match_string_with_gaps() helper Alexandru Ardelean
@ 2020-12-22 13:13 ` Alexandru Ardelean
  2020-12-22 13:42 ` [PATCH v6 1/2] lib/string.c: add __sysfs_match_string_with_gaps() helper Andy Shevchenko
  1 sibling, 0 replies; 4+ messages in thread
From: Alexandru Ardelean @ 2020-12-22 13:13 UTC (permalink / raw)
  To: linux-iio, linux-kernel
  Cc: jic23, lars, akpm, gregkh, andy.shevchenko, Alexandru Ardelean

From: Lars-Peter Clausen <lars@metafoo.de>

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 can not be used.

Add support for such enums to the IIO enum helper functions. A reserved
values is marked by setting its entry in the items array to NULL rather
than the normal descriptive string value.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/iio/industrialio-core.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index e9ee9363fed0..5f6211bfb428 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -470,8 +470,11 @@ 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';
@@ -492,7 +495,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]);
@@ -509,7 +512,7 @@ ssize_t iio_enum_write(struct iio_dev *indio_dev,
 	if (!e->set)
 		return -EINVAL;
 
-	ret = __sysfs_match_string(e->items, e->num_items, buf);
+	ret = __sysfs_match_string_with_gaps(e->items, e->num_items, buf);
 	if (ret < 0)
 		return ret;
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v6 1/2] lib/string.c: add __sysfs_match_string_with_gaps() helper
  2020-12-22 13:13 [PATCH v6 1/2] lib/string.c: add __sysfs_match_string_with_gaps() helper Alexandru Ardelean
  2020-12-22 13:13 ` [PATCH v6 2/2] iio: Handle enumerated properties with gaps Alexandru Ardelean
@ 2020-12-22 13:42 ` Andy Shevchenko
  2020-12-23  7:31   ` Alexandru Ardelean
  1 sibling, 1 reply; 4+ messages in thread
From: Andy Shevchenko @ 2020-12-22 13:42 UTC (permalink / raw)
  To: Alexandru Ardelean
  Cc: linux-iio, Linux Kernel Mailing List, Jonathan Cameron,
	Lars-Peter Clausen, Andrew Morton, Greg Kroah-Hartman

On Tue, Dec 22, 2020 at 3:09 PM Alexandru Ardelean
<alexandru.ardelean@analog.com> wrote:
>
> The original docstring of the __sysfs_match_string() and match_string()
> helper, implied that -1 could be used to search through NULL terminated
> arrays, and positive 'n' could be used to go through arrays that may have
> NULL elements in the middle of the array.
>
> This isn't true. Regardless of the value of 'n', the first NULL element in
> the array will stop the search, even if the element may be after a NULL
> element.
>
> To allow for a behavior where we can use the __sysfs_match_string() to
> search over arrays with NULL elements in the middle, the
> __sysfs_match_string_with_gaps() helper is added.
> If n > 0, the search will continue until the element is found or n is
> reached.
> If n < 0, the search will continue until the element is found or a NULL
> character is found.

I'm wondering if we can leave __sysfs_match_string() alone (w/o adding
unnecessary branch).

int __sysfs_match_string_with_gaps(const char * const *array, size_t
n, const char *str)
{
       const char *item;
       int index;

       for (index = 0; index < n; index++) {
               item = array[index];
               if (!item)
                       continue;
               if (sysfs_streq(item, str))
                       return index;
       }
       return -EINVAL;
}

Note, the check n>0 seems redundant for this particular function.

> +static int __sysfs_match_string_common(const char * const *array, ssize_t n,
> +                                      const char *str, bool gaps)
> +{
> +       const char *item;
> +       int index;
> +
> +       for (index = 0; index < n; index++) {
> +               item = array[index];
> +               if (!item) {
> +                       if (gaps && n > 0)
> +                               continue;
> +                       break;
> +               }
> +               if (sysfs_streq(item, str))
> +                       return index;
> +       }
> +
> +       return -EINVAL;
> +}
> +
>  /**
>   * __sysfs_match_string - matches given string in an array
>   * @array: array of strings
> @@ -770,21 +790,32 @@ EXPORT_SYMBOL(match_string);
>   */
>  int __sysfs_match_string(const char * const *array, size_t n, const char *str)
>  {
> -       const char *item;
> -       int index;
> -
> -       for (index = 0; index < n; index++) {
> -               item = array[index];
> -               if (!item)
> -                       break;
> -               if (sysfs_streq(item, str))
> -                       return index;
> -       }
> -
> -       return -EINVAL;
> +       return __sysfs_match_string_common(array, n, str, false);
>  }

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v6 1/2] lib/string.c: add __sysfs_match_string_with_gaps() helper
  2020-12-22 13:42 ` [PATCH v6 1/2] lib/string.c: add __sysfs_match_string_with_gaps() helper Andy Shevchenko
@ 2020-12-23  7:31   ` Alexandru Ardelean
  0 siblings, 0 replies; 4+ messages in thread
From: Alexandru Ardelean @ 2020-12-23  7:31 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Alexandru Ardelean, linux-iio, Linux Kernel Mailing List,
	Jonathan Cameron, Lars-Peter Clausen, Andrew Morton,
	Greg Kroah-Hartman

On Tue, Dec 22, 2020 at 3:43 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Tue, Dec 22, 2020 at 3:09 PM Alexandru Ardelean
> <alexandru.ardelean@analog.com> wrote:
> >
> > The original docstring of the __sysfs_match_string() and match_string()
> > helper, implied that -1 could be used to search through NULL terminated
> > arrays, and positive 'n' could be used to go through arrays that may have
> > NULL elements in the middle of the array.
> >
> > This isn't true. Regardless of the value of 'n', the first NULL element in
> > the array will stop the search, even if the element may be after a NULL
> > element.
> >
> > To allow for a behavior where we can use the __sysfs_match_string() to
> > search over arrays with NULL elements in the middle, the
> > __sysfs_match_string_with_gaps() helper is added.
> > If n > 0, the search will continue until the element is found or n is
> > reached.
> > If n < 0, the search will continue until the element is found or a NULL
> > character is found.
>
> I'm wondering if we can leave __sysfs_match_string() alone (w/o adding
> unnecessary branch).

Works for me.
Will re-spin.

>
> int __sysfs_match_string_with_gaps(const char * const *array, size_t
> n, const char *str)
> {
>        const char *item;
>        int index;
>
>        for (index = 0; index < n; index++) {
>                item = array[index];
>                if (!item)
>                        continue;
>                if (sysfs_streq(item, str))
>                        return index;
>        }
>        return -EINVAL;
> }
>
> Note, the check n>0 seems redundant for this particular function.
>
> > +static int __sysfs_match_string_common(const char * const *array, ssize_t n,
> > +                                      const char *str, bool gaps)
> > +{
> > +       const char *item;
> > +       int index;
> > +
> > +       for (index = 0; index < n; index++) {
> > +               item = array[index];
> > +               if (!item) {
> > +                       if (gaps && n > 0)
> > +                               continue;
> > +                       break;
> > +               }
> > +               if (sysfs_streq(item, str))
> > +                       return index;
> > +       }
> > +
> > +       return -EINVAL;
> > +}
> > +
> >  /**
> >   * __sysfs_match_string - matches given string in an array
> >   * @array: array of strings
> > @@ -770,21 +790,32 @@ EXPORT_SYMBOL(match_string);
> >   */
> >  int __sysfs_match_string(const char * const *array, size_t n, const char *str)
> >  {
> > -       const char *item;
> > -       int index;
> > -
> > -       for (index = 0; index < n; index++) {
> > -               item = array[index];
> > -               if (!item)
> > -                       break;
> > -               if (sysfs_streq(item, str))
> > -                       return index;
> > -       }
> > -
> > -       return -EINVAL;
> > +       return __sysfs_match_string_common(array, n, str, false);
> >  }
>
> --
> With Best Regards,
> Andy Shevchenko

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-12-23  7:32 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-22 13:13 [PATCH v6 1/2] lib/string.c: add __sysfs_match_string_with_gaps() helper Alexandru Ardelean
2020-12-22 13:13 ` [PATCH v6 2/2] iio: Handle enumerated properties with gaps Alexandru Ardelean
2020-12-22 13:42 ` [PATCH v6 1/2] lib/string.c: add __sysfs_match_string_with_gaps() helper Andy Shevchenko
2020-12-23  7:31   ` Alexandru Ardelean

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).