linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Rasmus Villemoes <linux@rasmusvillemoes.dk>
To: Gioh Kim <gi-oh.kim@ionos.com>,
	Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Nick Desaulniers <ndesaulniers@google.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: Re: [PATCH] lib/string: sysfs_streq works case insensitively
Date: Wed, 28 Apr 2021 09:47:13 +0200	[thread overview]
Message-ID: <650dc1b8-d801-2263-2e5c-eb833f2c4534@rasmusvillemoes.dk> (raw)
In-Reply-To: <CAJX1YtYRK=_X8+mvva2S35-K4kpwXSAGctUJ01TEDFRhS0y5LA@mail.gmail.com>

On 28/04/2021 09.31, Gioh Kim wrote:
> On Wed, Apr 28, 2021 at 8:42 AM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
>>

>>
>> Are you sure it’s good change? Sysfs is used for an ABI and you are opening a can of worms. From me NAK to this change without a very good background description that tells why it is safe to do.
> 
> https://www.spinics.net/lists/kernel/msg3898123.html
> My initial idea was making a new function: sysfs_streqcase.
> Andrew and Greg suggested making sysfs_streq to be case-insensitive.
> I would like to have a discussion about it.

1. That information should be in the commit log, not some random
babbling about case sensitivity of file systems.

2. So as Andy says, this is changing ABI for a whole lot of users in one
go. While it's _probably_ true that nobody would care (because it just
ends up accepting more strings, not fewer), your motivation seems to be
to replace uses of strncasecmp() to prevent "disableGARBAGE@#$@#@" to be
accepted as equivalent to "disable". I.e., those potential new users of
sysfs_streq() would have their ABI changed towards being less
permissive. That's a bigger change, with a higher chance of breaking
something. Do you even know if the maintainers of those drivers would
accept a switch to a case-insensitive sysfs_streq()?

Sorry, I really think you need a lot stronger motivation for introducing
either this change or a sysfs_strcaseeq().

Rasmus

  parent reply	other threads:[~2021-04-28  7:47 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-12 11:33 [PATCH] lib/string: sysfs_streq works case insensitively Gioh Kim
2021-04-28  5:41 ` Gioh Kim
     [not found]   ` <CAHp75Vf2yJ5=zdxRcPKmKGCKeF8As=Nv2S9fm0ciVXL5HGbWDg@mail.gmail.com>
2021-04-28  7:31     ` Gioh Kim
2021-04-28  7:46       ` Andy Shevchenko
2021-04-28  7:47       ` Rasmus Villemoes [this message]
2021-04-28  8:31         ` Gioh Kim
2021-04-28  9:13           ` Rasmus Villemoes
2021-04-28  6:37 ` Rasmus Villemoes

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=650dc1b8-d801-2263-2e5c-eb833f2c4534@rasmusvillemoes.dk \
    --to=linux@rasmusvillemoes.dk \
    --cc=akpm@linux-foundation.org \
    --cc=andy.shevchenko@gmail.com \
    --cc=gi-oh.kim@ionos.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ndesaulniers@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).