Re: [PATCH v4] lib/string: Introduce sysfs_streqcase

[Nick Desaulniers]

On Thu, Apr 8, 2021 at 7:52 AM Gioh Kim <gi-oh.kim@xxxxxxxxx> wrote:
>
> On Thu, Apr 8, 2021 at 3:14 PM Jinpu Wang <jinpu.wang@xxxxxxxxx> wrote:
> >
> > On Thu, Apr 8, 2021 at 3:06 PM Gioh Kim <gi-oh.kim@xxxxxxxxx> wrote:
> > >
> > > As the name shows, it checks if strings are equal in case insensitive
> > > manner.
> > >
> > > For example, drivers/infiniband/ulp/rtrs/rtrs-clt-sysfs.c uses
> > > strncasecmp to check that the input via sysfs is "mi". But it would
> > > work even-if the input is "min-wrongcommand".
> > >
> > > I found some more cases using strncasecmp to check the entire string
> > > such as rtrs-clt-sysfs.c does. drivers/pnp/interface.c checks
> > > "disable" command with strncasecmp but it would also work if the
> > > command is "disable-wrong".
> > >
> > > Signed-off-by: Gioh Kim <gi-oh.kim@xxxxxxxxx>

v4 LGTM, thanks.

Reviewed-by: Nick Desaulniers <ndesaulniers@xxxxxxxxxx>

> > you should add the
> > Reported-by: kernel test robot <lkp@xxxxxxxxx>
> > > ---
> > you can add the changelog here after the ---
> > v4->v3:  removed #ifdef CONFIG_SYSFS ~ #endif.
> >
> > The string comparison doesn't depends on CONFIG_SYSFS at all.
> >
> > It looks good to me.
> > Reviewed-by: Jack Wang <jinpu.wang@xxxxxxxxx>
> >
> >
>
> Yes, I got two build error reports for v3.
> Should I send v5 including "Reported-by: kernel test robot <lkp@xxxxxxxxx>" tag?

I don't think that's necessary.  I would use that tag if I was fixing
an issue reported by the bot; but v4 is basically the same as v2 in
regards to the issue 0day bot reported with v3. v3 just demonstrates
that there are drivers with possibly incorrect Kconfig dependencies
(missing a dependency on SYSFS perhaps). So the underlying problem was
not reported by 0day bot; 0day bot just helped avoid issues from v3.

Fixing the Kconfig dependencies would be nice to have, but not a
requirement IMO to this feature/functionality.

-- 
Thanks,
~Nick Desaulniers