linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4] lib/string: Introduce sysfs_streqcase
@ 2021-04-08 13:06 Gioh Kim
  2021-04-08 13:13 ` Jinpu Wang
  2021-04-09  5:05 ` Andrew Morton
  0 siblings, 2 replies; 9+ messages in thread
From: Gioh Kim @ 2021-04-08 13:06 UTC (permalink / raw)
  To: linux-kernel
  Cc: ndesaulniers, dan.j.williams, laniel_francis, keescook, dja,
	akpm, haris.iqbal, jinpu.wang, Gioh Kim

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@ionos.com>
---
 include/linux/string.h |  1 +
 lib/string.c           | 36 ++++++++++++++++++++++++++++--------
 2 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/include/linux/string.h b/include/linux/string.h
index 4fcfb56abcf5..36d00ff8013e 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -184,6 +184,7 @@ extern char **argv_split(gfp_t gfp, const char *str, int *argcp);
 extern void argv_free(char **argv);
 
 extern bool sysfs_streq(const char *s1, const char *s2);
+extern bool sysfs_streqcase(const char *s1, const char *s2);
 extern int kstrtobool(const char *s, bool *res);
 static inline int strtobool(const char *s, bool *res)
 {
diff --git a/lib/string.c b/lib/string.c
index 7548eb715ddb..d0fb02efd5da 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -687,6 +687,17 @@ char *strsep(char **s, const char *ct)
 EXPORT_SYMBOL(strsep);
 #endif
 
+static inline bool __streq_terminal(const char *s1, const char *s2)
+{
+	if (*s1 == *s2)
+		return true;
+	if (!*s1 && *s2 == '\n' && !s2[1])
+		return true;
+	if (*s1 == '\n' && !s1[1] && !*s2)
+		return true;
+	return false;
+}
+
 /**
  * sysfs_streq - return true if strings are equal, modulo trailing newline
  * @s1: one string
@@ -703,17 +714,26 @@ bool sysfs_streq(const char *s1, const char *s2)
 		s1++;
 		s2++;
 	}
-
-	if (*s1 == *s2)
-		return true;
-	if (!*s1 && *s2 == '\n' && !s2[1])
-		return true;
-	if (*s1 == '\n' && !s1[1] && !*s2)
-		return true;
-	return false;
+	return __streq_terminal(s1, s2);
 }
 EXPORT_SYMBOL(sysfs_streq);
 
+/**
+ * sysfs_streqcase - same to sysfs_streq and case insensitive
+ * @s1: one string
+ * @s2: another string
+ *
+ */
+bool sysfs_streqcase(const char *s1, const char *s2)
+{
+	while (*s1 && tolower(*s1) == tolower(*s2)) {
+		s1++;
+		s2++;
+	}
+	return __streq_terminal(s1, s2);
+}
+EXPORT_SYMBOL(sysfs_streqcase);
+
 /**
  * match_string - matches given string in an array
  * @array:	array of strings
-- 
2.25.1


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

* Re: [PATCH v4] lib/string: Introduce sysfs_streqcase
  2021-04-08 13:06 [PATCH v4] lib/string: Introduce sysfs_streqcase Gioh Kim
@ 2021-04-08 13:13 ` Jinpu Wang
  2021-04-08 14:52   ` Gioh Kim
  2021-04-09  5:05 ` Andrew Morton
  1 sibling, 1 reply; 9+ messages in thread
From: Jinpu Wang @ 2021-04-08 13:13 UTC (permalink / raw)
  To: Gioh Kim
  Cc: open list, Nick Desaulniers, Dan Williams, laniel_francis,
	Kees Cook, dja, Andrew Morton, Haris Iqbal

On Thu, Apr 8, 2021 at 3:06 PM Gioh Kim <gi-oh.kim@ionos.com> 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@ionos.com>
you should add the
Reported-by: kernel test robot <lkp@intel.com>
> ---
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@ionos.com>



>  include/linux/string.h |  1 +
>  lib/string.c           | 36 ++++++++++++++++++++++++++++--------
>  2 files changed, 29 insertions(+), 8 deletions(-)
>
> diff --git a/include/linux/string.h b/include/linux/string.h
> index 4fcfb56abcf5..36d00ff8013e 100644
> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -184,6 +184,7 @@ extern char **argv_split(gfp_t gfp, const char *str, int *argcp);
>  extern void argv_free(char **argv);
>
>  extern bool sysfs_streq(const char *s1, const char *s2);
> +extern bool sysfs_streqcase(const char *s1, const char *s2);
>  extern int kstrtobool(const char *s, bool *res);
>  static inline int strtobool(const char *s, bool *res)
>  {
> diff --git a/lib/string.c b/lib/string.c
> index 7548eb715ddb..d0fb02efd5da 100644
> --- a/lib/string.c
> +++ b/lib/string.c
> @@ -687,6 +687,17 @@ char *strsep(char **s, const char *ct)
>  EXPORT_SYMBOL(strsep);
>  #endif
>
> +static inline bool __streq_terminal(const char *s1, const char *s2)
> +{
> +       if (*s1 == *s2)
> +               return true;
> +       if (!*s1 && *s2 == '\n' && !s2[1])
> +               return true;
> +       if (*s1 == '\n' && !s1[1] && !*s2)
> +               return true;
> +       return false;
> +}
> +
>  /**
>   * sysfs_streq - return true if strings are equal, modulo trailing newline
>   * @s1: one string
> @@ -703,17 +714,26 @@ bool sysfs_streq(const char *s1, const char *s2)
>                 s1++;
>                 s2++;
>         }
> -
> -       if (*s1 == *s2)
> -               return true;
> -       if (!*s1 && *s2 == '\n' && !s2[1])
> -               return true;
> -       if (*s1 == '\n' && !s1[1] && !*s2)
> -               return true;
> -       return false;
> +       return __streq_terminal(s1, s2);
>  }
>  EXPORT_SYMBOL(sysfs_streq);
>
> +/**
> + * sysfs_streqcase - same to sysfs_streq and case insensitive
> + * @s1: one string
> + * @s2: another string
> + *
> + */
> +bool sysfs_streqcase(const char *s1, const char *s2)
> +{
> +       while (*s1 && tolower(*s1) == tolower(*s2)) {
> +               s1++;
> +               s2++;
> +       }
> +       return __streq_terminal(s1, s2);
> +}
> +EXPORT_SYMBOL(sysfs_streqcase);
> +
>  /**
>   * match_string - matches given string in an array
>   * @array:     array of strings
> --
> 2.25.1
>

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

* Re: [PATCH v4] lib/string: Introduce sysfs_streqcase
  2021-04-08 13:13 ` Jinpu Wang
@ 2021-04-08 14:52   ` Gioh Kim
  2021-04-08 18:16     ` Nick Desaulniers
  0 siblings, 1 reply; 9+ messages in thread
From: Gioh Kim @ 2021-04-08 14:52 UTC (permalink / raw)
  To: Jinpu Wang
  Cc: open list, Nick Desaulniers, Dan Williams, laniel_francis,
	Kees Cook, Daniel Axtens, Andrew Morton, Haris Iqbal

On Thu, Apr 8, 2021 at 3:14 PM Jinpu Wang <jinpu.wang@ionos.com> wrote:
>
> On Thu, Apr 8, 2021 at 3:06 PM Gioh Kim <gi-oh.kim@ionos.com> 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@ionos.com>
> you should add the
> Reported-by: kernel test robot <lkp@intel.com>
> > ---
> 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@ionos.com>
>
>

Yes, I got two build error reports for v3.
Should I send v5 including "Reported-by: kernel test robot <lkp@intel.com>" tag?

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

* Re: [PATCH v4] lib/string: Introduce sysfs_streqcase
  2021-04-08 14:52   ` Gioh Kim
@ 2021-04-08 18:16     ` Nick Desaulniers
  0 siblings, 0 replies; 9+ messages in thread
From: Nick Desaulniers @ 2021-04-08 18:16 UTC (permalink / raw)
  To: Gioh Kim
  Cc: Jinpu Wang, open list, Dan Williams, laniel_francis, Kees Cook,
	Daniel Axtens, Andrew Morton, Haris Iqbal

On Thu, Apr 8, 2021 at 7:52 AM Gioh Kim <gi-oh.kim@ionos.com> wrote:
>
> On Thu, Apr 8, 2021 at 3:14 PM Jinpu Wang <jinpu.wang@ionos.com> wrote:
> >
> > On Thu, Apr 8, 2021 at 3:06 PM Gioh Kim <gi-oh.kim@ionos.com> 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@ionos.com>

v4 LGTM, thanks.

Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

> > you should add the
> > Reported-by: kernel test robot <lkp@intel.com>
> > > ---
> > 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@ionos.com>
> >
> >
>
> Yes, I got two build error reports for v3.
> Should I send v5 including "Reported-by: kernel test robot <lkp@intel.com>" 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

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

* Re: [PATCH v4] lib/string: Introduce sysfs_streqcase
  2021-04-08 13:06 [PATCH v4] lib/string: Introduce sysfs_streqcase Gioh Kim
  2021-04-08 13:13 ` Jinpu Wang
@ 2021-04-09  5:05 ` Andrew Morton
  2021-04-09  6:44   ` Greg Kroah-Hartman
       [not found]   ` <CAHp75VcjNWycV-SLUzzfgtMp8BzXUiUAoz_BYr4+nCazAzsrqw@mail.gmail.com>
  1 sibling, 2 replies; 9+ messages in thread
From: Andrew Morton @ 2021-04-09  5:05 UTC (permalink / raw)
  To: Gioh Kim
  Cc: linux-kernel, ndesaulniers, dan.j.williams, laniel_francis,
	keescook, dja, haris.iqbal, jinpu.wang, Greg Kroah-Hartman

On Thu,  8 Apr 2021 15:06:05 +0200 Gioh Kim <gi-oh.kim@ionos.com> wrote:

> As the name shows, it checks if strings are equal in case insensitive
> manner.

Peh.  Who would die if we simply made sysfs_streq() case-insensitive?

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

* Re: [PATCH v4] lib/string: Introduce sysfs_streqcase
  2021-04-09  5:05 ` Andrew Morton
@ 2021-04-09  6:44   ` Greg Kroah-Hartman
  2021-04-09  6:51     ` Andrew Morton
       [not found]   ` <CAHp75VcjNWycV-SLUzzfgtMp8BzXUiUAoz_BYr4+nCazAzsrqw@mail.gmail.com>
  1 sibling, 1 reply; 9+ messages in thread
From: Greg Kroah-Hartman @ 2021-04-09  6:44 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Gioh Kim, linux-kernel, ndesaulniers, dan.j.williams,
	laniel_francis, keescook, dja, haris.iqbal, jinpu.wang

On Thu, Apr 08, 2021 at 10:05:02PM -0700, Andrew Morton wrote:
> On Thu,  8 Apr 2021 15:06:05 +0200 Gioh Kim <gi-oh.kim@ionos.com> wrote:
> 
> > As the name shows, it checks if strings are equal in case insensitive
> > manner.
> 
> Peh.  Who would die if we simply made sysfs_streq() case-insensitive?

I doubt anyone, let's do that instead.

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

* Re: [PATCH v4] lib/string: Introduce sysfs_streqcase
  2021-04-09  6:44   ` Greg Kroah-Hartman
@ 2021-04-09  6:51     ` Andrew Morton
  2021-04-09  6:59       ` Greg Kroah-Hartman
  0 siblings, 1 reply; 9+ messages in thread
From: Andrew Morton @ 2021-04-09  6:51 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Gioh Kim, linux-kernel, ndesaulniers, dan.j.williams,
	laniel_francis, keescook, dja, haris.iqbal, jinpu.wang

On Fri, 9 Apr 2021 08:44:39 +0200 Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:

> On Thu, Apr 08, 2021 at 10:05:02PM -0700, Andrew Morton wrote:
> > On Thu,  8 Apr 2021 15:06:05 +0200 Gioh Kim <gi-oh.kim@ionos.com> wrote:
> > 
> > > As the name shows, it checks if strings are equal in case insensitive
> > > manner.
> > 
> > Peh.  Who would die if we simply made sysfs_streq() case-insensitive?
> 
> I doubt anyone, let's do that instead.

There's a risk that people will write scripts/config/etc on a 5.13+
kernel and then find that they malfunction on earlier kernels...


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

* Re: [PATCH v4] lib/string: Introduce sysfs_streqcase
  2021-04-09  6:51     ` Andrew Morton
@ 2021-04-09  6:59       ` Greg Kroah-Hartman
  0 siblings, 0 replies; 9+ messages in thread
From: Greg Kroah-Hartman @ 2021-04-09  6:59 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Gioh Kim, linux-kernel, ndesaulniers, dan.j.williams,
	laniel_francis, keescook, dja, haris.iqbal, jinpu.wang

On Thu, Apr 08, 2021 at 11:51:32PM -0700, Andrew Morton wrote:
> On Fri, 9 Apr 2021 08:44:39 +0200 Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> 
> > On Thu, Apr 08, 2021 at 10:05:02PM -0700, Andrew Morton wrote:
> > > On Thu,  8 Apr 2021 15:06:05 +0200 Gioh Kim <gi-oh.kim@ionos.com> wrote:
> > > 
> > > > As the name shows, it checks if strings are equal in case insensitive
> > > > manner.
> > > 
> > > Peh.  Who would die if we simply made sysfs_streq() case-insensitive?
> > 
> > I doubt anyone, let's do that instead.
> 
> There's a risk that people will write scripts/config/etc on a 5.13+
> kernel and then find that they malfunction on earlier kernels...
> 

That's not a regression, that is a "newer kernels have newer features"
issue :)

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

* Re: [PATCH v4] lib/string: Introduce sysfs_streqcase
       [not found]   ` <CAHp75VcjNWycV-SLUzzfgtMp8BzXUiUAoz_BYr4+nCazAzsrqw@mail.gmail.com>
@ 2021-04-09 12:41     ` Gioh Kim
  0 siblings, 0 replies; 9+ messages in thread
From: Gioh Kim @ 2021-04-09 12:41 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Andrew Morton, linux-kernel, ndesaulniers, dan.j.williams,
	laniel_francis, keescook, dja, haris.iqbal, jinpu.wang,
	Greg Kroah-Hartman

On Fri, Apr 9, 2021 at 9:11 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
>
>
> On Friday, April 9, 2021, Andrew Morton <akpm@linux-foundation.org> wrote:
>>
>> On Thu,  8 Apr 2021 15:06:05 +0200 Gioh Kim <gi-oh.kim@ionos.com> wrote:
>>
>> > As the name shows, it checks if strings are equal in case insensitive
>> > manner.
>>
>> Peh.  Who would die if we simply made sysfs_streq() case-insensitive?
>
>
> I think it will be a disaster. Like we have with case-insensitive file systems. Famous Mac case that most of the soft stop working when they actually moved to the right direction, I.e. case-sensitive. Personally I’m against such change.

Hi Andy,

I am sorry but I am a little bit confused because I don't have any
experience with case-insensitive file systems.
I think the case-insensitive file system does not care if sysfs_streq
is changed to case-insensitive.

For example, if the case-insensitive file system would accept the "OFF" command,
the user can pass either of "off" and "OFF" and both will work with
case-insensitive strfs_streq.

On the contrary, I think the case-sensitive file system would not like
it if sysfs_streq is made case-insensitive.

Could you please inform me what I am missing?

>
>
>
>
> --
> With Best Regards,
> Andy Shevchenko
>
>

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

end of thread, other threads:[~2021-04-09 12:41 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-08 13:06 [PATCH v4] lib/string: Introduce sysfs_streqcase Gioh Kim
2021-04-08 13:13 ` Jinpu Wang
2021-04-08 14:52   ` Gioh Kim
2021-04-08 18:16     ` Nick Desaulniers
2021-04-09  5:05 ` Andrew Morton
2021-04-09  6:44   ` Greg Kroah-Hartman
2021-04-09  6:51     ` Andrew Morton
2021-04-09  6:59       ` Greg Kroah-Hartman
     [not found]   ` <CAHp75VcjNWycV-SLUzzfgtMp8BzXUiUAoz_BYr4+nCazAzsrqw@mail.gmail.com>
2021-04-09 12:41     ` Gioh Kim

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