Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210107084434.35283-1-alexandru.ardelean@analog.com>
In-Reply-To: <20210107084434.35283-1-alexandru.ardelean@analog.com>
From:   Andy Shevchenko <andy.shevchenko@gmail.com>
Date:   Thu, 7 Jan 2021 12:09:32 +0200
Message-ID: <CAHp75Vd0+r+Yo1+mLbKcHqjfjEUtVoSEX7Kvjj4DO4osu44Wyw@mail.gmail.com>
Subject: Re: [PATCH v7 1/2] lib/string.c: add __sysfs_match_string_with_gaps() helper
To:     Alexandru Ardelean <alexandru.ardelean@analog.com>
Cc:     linux-iio <linux-iio@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Jonathan Cameron <jic23@kernel.org>,
        Lars-Peter Clausen <lars@metafoo.de>,
        Andrew Morton <akpm@linux-foundation.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, Jan 7, 2021 at 10:41 AM 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.

And this is what I think is not needed.

...

> FWIW (from my side): I am not fully sure yet that implementing a
> new helper is the best idea. It's also not a bad idea.
> It's one of those "this could be useful for others as well later", but
> right now we would have only one user (IIO) for this.

Taking into account the failure of bitmap_set_clump() promotion, I
suggest to reduce scope to IIO for now.

> I'm also fine with just implementing it in IIO first, and the someone
> else can move it to lib/string.c if needed.

Yes, please do.

...

> + * If n < 0, then the search will continue until the element is found or
> + * until the first NULL element.

...

> +                       if (n < 0)
> +                               break;

And drop these.

-- 
With Best Regards,
Andy Shevchenko