| Message ID | 20220425085233.82576-1-jvgediya@linux.ibm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [RESEND] string_helpers: sysfs: Add helper to get bool from string | expand |
On Mon, 25 Apr 2022 14:22:33 +0530 Jagdish Gediya <jvgediya@linux.ibm.com> wrote: > At many places in kernel, It is necessary to convert sysfs input > to corrosponding bool value e.g. "false" or "0" need to be converted > to bool false, "true" or "1" need to be converted to bool true, > places where such conversion is needed currently check the input > string manually. Also, such conversions compare sysfs input using > strncmp functions so even if certain number of character match in the > beginning, they assume the string as valid bool, which is not the > right semantic e.g. false is bool but falseX is not. > > Introduce new string helper function to convert sysfs input to > corrosponding bool value. Modify existing such conversions to use > this new function. > > logs, > $ cat /sys/kernel/mm/numa/demotion_enabled > false > $ echo true > /sys/kernel/mm/numa/demotion_enabled > $ cat demotion_enabled > true > $ echo truex > /sys/kernel/mm/numa/demotion_enabled > -bash: echo: write error: Invalid argument > $ echo 10 > /sys/kernel/mm/numa/demotion_enabled > -bash: echo: write error: Invalid argument > $ echo false > /sys/kernel/mm/numa/demotion_enabled > $ cat demotion_enabled > false > $ echo falseabc > /sys/kernel/mm/numa/demotion_enabled > -bash: echo: write error: Invalid argument > $ echo 1 > /sys/kernel/mm/numa/demotion_enabled > $ cat demotion_enabled > true > $ echo 0 > /sys/kernel/mm/numa/demotion_enabled > $ cat demotion_enabled > false > > This patch doesn't have any functionality change. > > ... > > --- a/lib/string_helpers.c > +++ b/lib/string_helpers.c > @@ -967,6 +967,26 @@ void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count, > } > EXPORT_SYMBOL(memcpy_and_pad); > > +/** > + * sysfs_strbool - Get bool value corrosponding to string > + * @s: The string to operate on. > + * @output: Pointer to fill resulting bool value > + * > + * Returns 1 if string represents bool value, 0 otherwise > + */ > +int sysfs_strbool(const char *s, bool *output) > +{ > + if (sysfs_streq(s, "1") || sysfs_streq(s, "true")) > + *output = true; > + else if (sysfs_streq(s, "0") || sysfs_streq(s, "false")) > + *output = false; > + else > + return 0; > + > + return 1; > +} > +EXPORT_SYMBOL(sysfs_strbool); > + Can we teach kstrtobool() about "true" and "false" then use that?
On Mon, Apr 25, 2022 at 03:00:46PM -0700, Andrew Morton wrote: > On Mon, 25 Apr 2022 14:22:33 +0530 Jagdish Gediya <jvgediya@linux.ibm.com> wrote: > > > At many places in kernel, It is necessary to convert sysfs input > > to corrosponding bool value e.g. "false" or "0" need to be converted > > to bool false, "true" or "1" need to be converted to bool true, > > places where such conversion is needed currently check the input > > string manually. Also, such conversions compare sysfs input using > > strncmp functions so even if certain number of character match in the > > beginning, they assume the string as valid bool, which is not the > > right semantic e.g. false is bool but falseX is not. > > > > Introduce new string helper function to convert sysfs input to > > corrosponding bool value. Modify existing such conversions to use > > this new function. > > > > logs, > > $ cat /sys/kernel/mm/numa/demotion_enabled > > false > > $ echo true > /sys/kernel/mm/numa/demotion_enabled > > $ cat demotion_enabled > > true > > $ echo truex > /sys/kernel/mm/numa/demotion_enabled > > -bash: echo: write error: Invalid argument > > $ echo 10 > /sys/kernel/mm/numa/demotion_enabled > > -bash: echo: write error: Invalid argument > > $ echo false > /sys/kernel/mm/numa/demotion_enabled > > $ cat demotion_enabled > > false > > $ echo falseabc > /sys/kernel/mm/numa/demotion_enabled > > -bash: echo: write error: Invalid argument > > $ echo 1 > /sys/kernel/mm/numa/demotion_enabled > > $ cat demotion_enabled > > true > > $ echo 0 > /sys/kernel/mm/numa/demotion_enabled > > $ cat demotion_enabled > > false > > > > This patch doesn't have any functionality change. > > > > ... > > > > --- a/lib/string_helpers.c > > +++ b/lib/string_helpers.c > > @@ -967,6 +967,26 @@ void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count, > > } > > EXPORT_SYMBOL(memcpy_and_pad); > > > > +/** > > + * sysfs_strbool - Get bool value corrosponding to string > > + * @s: The string to operate on. > > + * @output: Pointer to fill resulting bool value > > + * > > + * Returns 1 if string represents bool value, 0 otherwise > > + */ > > +int sysfs_strbool(const char *s, bool *output) > > +{ > > + if (sysfs_streq(s, "1") || sysfs_streq(s, "true")) > > + *output = true; > > + else if (sysfs_streq(s, "0") || sysfs_streq(s, "false")) > > + *output = false; > > + else > > + return 0; > > + > > + return 1; > > +} > > +EXPORT_SYMBOL(sysfs_strbool); > > + > > Can we teach kstrtobool() about "true" and "false" then use that? I think we can do that, I have sent v2. Thanks
diff --git a/include/linux/string.h b/include/linux/string.h index b6572aeca2f5..3c00991b456a 100644 --- a/include/linux/string.h +++ b/include/linux/string.h @@ -170,6 +170,7 @@ static inline void memcpy_flushcache(void *dst, const void *src, size_t cnt) void *memchr_inv(const void *s, int c, size_t n); char *strreplace(char *s, char old, char new); +int sysfs_strbool(const char *s, bool *output); extern void kfree_const(const void *x); diff --git a/lib/string_helpers.c b/lib/string_helpers.c index 4f877e9551d5..cd3580b7ea06 100644 --- a/lib/string_helpers.c +++ b/lib/string_helpers.c @@ -967,6 +967,26 @@ void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count, } EXPORT_SYMBOL(memcpy_and_pad); +/** + * sysfs_strbool - Get bool value corrosponding to string + * @s: The string to operate on. + * @output: Pointer to fill resulting bool value + * + * Returns 1 if string represents bool value, 0 otherwise + */ +int sysfs_strbool(const char *s, bool *output) +{ + if (sysfs_streq(s, "1") || sysfs_streq(s, "true")) + *output = true; + else if (sysfs_streq(s, "0") || sysfs_streq(s, "false")) + *output = false; + else + return 0; + + return 1; +} +EXPORT_SYMBOL(sysfs_strbool); + #ifdef CONFIG_FORTIFY_SOURCE /* These are placeholders for fortify compile-time warnings. */ void __read_overflow2_field(size_t avail, size_t wanted) { } diff --git a/mm/migrate.c b/mm/migrate.c index 6c31ee1e1c9b..a5b105144016 100644 --- a/mm/migrate.c +++ b/mm/migrate.c @@ -49,6 +49,7 @@ #include <linux/oom.h> #include <linux/memory.h> #include <linux/random.h> +#include <linux/string.h> #include <linux/sched/sysctl.h> #include <asm/tlbflush.h> @@ -2523,11 +2524,7 @@ static ssize_t numa_demotion_enabled_store(struct kobject *kobj, struct kobj_attribute *attr, const char *buf, size_t count) { - if (!strncmp(buf, "true", 4) || !strncmp(buf, "1", 1)) - numa_demotion_enabled = true; - else if (!strncmp(buf, "false", 5) || !strncmp(buf, "0", 1)) - numa_demotion_enabled = false; - else + if (!sysfs_strbool(buf, &numa_demotion_enabled)) return -EINVAL; return count; diff --git a/mm/swap_state.c b/mm/swap_state.c index 013856004825..4f439845d176 100644 --- a/mm/swap_state.c +++ b/mm/swap_state.c @@ -865,11 +865,7 @@ static ssize_t vma_ra_enabled_store(struct kobject *kobj, struct kobj_attribute *attr, const char *buf, size_t count) { - if (!strncmp(buf, "true", 4) || !strncmp(buf, "1", 1)) - enable_vma_readahead = true; - else if (!strncmp(buf, "false", 5) || !strncmp(buf, "0", 1)) - enable_vma_readahead = false; - else + if (!sysfs_strbool(buf, &enable_vma_readahead)) return -EINVAL; return count;
At many places in kernel, It is necessary to convert sysfs input to corrosponding bool value e.g. "false" or "0" need to be converted to bool false, "true" or "1" need to be converted to bool true, places where such conversion is needed currently check the input string manually. Also, such conversions compare sysfs input using strncmp functions so even if certain number of character match in the beginning, they assume the string as valid bool, which is not the right semantic e.g. false is bool but falseX is not. Introduce new string helper function to convert sysfs input to corrosponding bool value. Modify existing such conversions to use this new function. logs, $ cat /sys/kernel/mm/numa/demotion_enabled false $ echo true > /sys/kernel/mm/numa/demotion_enabled $ cat demotion_enabled true $ echo truex > /sys/kernel/mm/numa/demotion_enabled -bash: echo: write error: Invalid argument $ echo 10 > /sys/kernel/mm/numa/demotion_enabled -bash: echo: write error: Invalid argument $ echo false > /sys/kernel/mm/numa/demotion_enabled $ cat demotion_enabled false $ echo falseabc > /sys/kernel/mm/numa/demotion_enabled -bash: echo: write error: Invalid argument $ echo 1 > /sys/kernel/mm/numa/demotion_enabled $ cat demotion_enabled true $ echo 0 > /sys/kernel/mm/numa/demotion_enabled $ cat demotion_enabled false This patch doesn't have any functionality change. Signed-off-by: Jagdish Gediya <jvgediya@linux.ibm.com> --- Resending as initially forgot to add linux-mm in recipients. include/linux/string.h | 1 + lib/string_helpers.c | 20 ++++++++++++++++++++ mm/migrate.c | 7 ++----- mm/swap_state.c | 6 +----- 4 files changed, 24 insertions(+), 10 deletions(-)