Re: [PATCH v6 1/2] lib/string.c: add __sysfs_match_string_with_gaps() helper

From: Alexandru Ardelean
Date: Wed Dec 23 2020 - 02:33:45 EST


On Tue, Dec 22, 2020 at 3:43 PM Andy Shevchenko
<andy.shevchenko@xxxxxxxxx> wrote:
>
> On Tue, Dec 22, 2020 at 3:09 PM Alexandru Ardelean
> <alexandru.ardelean@xxxxxxxxxx> 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