Messages in this thread | | | From | Gioh Kim <> | Date | Thu, 8 Apr 2021 09:25:29 +0200 | Subject | Re: [PATCH] lib/string: Introduce sysfs_streqcase |
| |
On Wed, Apr 7, 2021 at 10:07 PM Nick Desaulniers <ndesaulniers@google.com> wrote: > > On Tue, Apr 6, 2021 at 11:15 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". > > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > > I do wonder if these (sysfs_streqcase and sysfs_streq) could or should > be conditionally available on CONFIG_SYSFS=y; don't pay for what you > don't use (without needing CONFIG_LD_DEAD_CODE_DATA_ELIMINATION=y)?
Good idea. Thank you.
> > Also, it might be nice to share the second half of the function with > sysfs_streq via a new static function. Though it will just get > inlined in both for CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE=y, it might > help the compiler if CONFIG_CC_OPTIMIZE_FOR_SIZE=y was instead chosen > if the compiler cannot outline/deduplicate the shared code. At the > least, there's less duplication between two very similar functions; if > one changes then authors may need to be careful to update both.
Yes, they are exactly the same. I will make an inline function for the common code.
> > Are either of those concerns worth a v3? ¯\_(ツ)_/¯
Sure, I will not forget to add 'V2'.
Thank you for kind review.
| |