Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1327193 > unrolled thread

[PATCH v2 1/4] lib: move strtobool to kstrtobool

Started byKees Cook <keescook@chromium.org>
First post2016-02-04 22:10 +0100
Last post2016-02-05 22:00 +0100
Articles 5 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v2 1/4] lib: move strtobool to kstrtobool Kees Cook <keescook@chromium.org> - 2016-02-04 22:10 +0100
    Re: [PATCH v2 1/4] lib: move strtobool to kstrtobool Andy Shevchenko <andy.shevchenko@gmail.com> - 2016-02-04 23:50 +0100
      Re: [PATCH v2 1/4] lib: move strtobool to kstrtobool Kees Cook <keescook@chromium.org> - 2016-02-05 00:00 +0100
    Re: [PATCH v2 1/4] lib: move strtobool to kstrtobool Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2016-02-05 01:00 +0100
      Re: [PATCH v2 1/4] lib: move strtobool to kstrtobool Kees Cook <keescook@chromium.org> - 2016-02-05 22:00 +0100

#1327193 — [PATCH v2 1/4] lib: move strtobool to kstrtobool

FromKees Cook <keescook@chromium.org>
Date2016-02-04 22:10 +0100
Subject[PATCH v2 1/4] lib: move strtobool to kstrtobool
Message-ID<qYzk5-7oW-11@gated-at.bofh.it>
Create the kstrtobool_from_user helper and moves strtobool logic into
the new kstrtobool (matching all the other kstrto* functions). Provides
an inline wrapper for existing strtobool callers.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 include/linux/kernel.h |  3 +++
 include/linux/string.h |  6 +++++-
 lib/kstrtox.c          | 35 +++++++++++++++++++++++++++++++++++
 lib/string.c           | 29 -----------------------------
 4 files changed, 43 insertions(+), 30 deletions(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index f31638c6e873..cdc25f47a23f 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -357,6 +357,7 @@ int __must_check kstrtou16(const char *s, unsigned int base, u16 *res);
 int __must_check kstrtos16(const char *s, unsigned int base, s16 *res);
 int __must_check kstrtou8(const char *s, unsigned int base, u8 *res);
 int __must_check kstrtos8(const char *s, unsigned int base, s8 *res);
+int __must_check kstrtobool(const char *s, unsigned int base, bool *res);
 
 int __must_check kstrtoull_from_user(const char __user *s, size_t count, unsigned int base, unsigned long long *res);
 int __must_check kstrtoll_from_user(const char __user *s, size_t count, unsigned int base, long long *res);
@@ -368,6 +369,8 @@ int __must_check kstrtou16_from_user(const char __user *s, size_t count, unsigne
 int __must_check kstrtos16_from_user(const char __user *s, size_t count, unsigned int base, s16 *res);
 int __must_check kstrtou8_from_user(const char __user *s, size_t count, unsigned int base, u8 *res);
 int __must_check kstrtos8_from_user(const char __user *s, size_t count, unsigned int base, s8 *res);
+int __must_check kstrtobool_from_user(const char __user *s, size_t count,
+				      unsigned int base, bool *res);
 
 static inline int __must_check kstrtou64_from_user(const char __user *s, size_t count, unsigned int base, u64 *res)
 {
diff --git a/include/linux/string.h b/include/linux/string.h
index 9eebc66d957a..d2fb21b1081d 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -128,7 +128,11 @@ 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 int strtobool(const char *s, bool *res);
+extern int kstrtobool(const char *s, unsigned int base, bool *res);
+static inline int strtobool(const char *s, bool *res)
+{
+	return kstrtobool(s, 0, res);
+}
 
 #ifdef CONFIG_BINARY_PRINTF
 int vbin_printf(u32 *bin_buf, size_t size, const char *fmt, va_list args);
diff --git a/lib/kstrtox.c b/lib/kstrtox.c
index 94be244e8441..e18f088704d7 100644
--- a/lib/kstrtox.c
+++ b/lib/kstrtox.c
@@ -321,6 +321,40 @@ int kstrtos8(const char *s, unsigned int base, s8 *res)
 }
 EXPORT_SYMBOL(kstrtos8);
 
+/**
+ * kstrtobool - convert common user inputs into boolean values
+ * @s: input string
+ * @base: ignored
+ * @res: result
+ *
+ * This routine returns 0 iff the first character is one of 'Yy1Nn0'.
+ * Otherwise it will return -EINVAL.  Value pointed to by res is
+ * updated upon finding a match.
+ */
+int kstrtobool(const char *s, unsigned int base, bool *res)
+{
+	if (!s)
+		return -EINVAL;
+
+	switch (s[0]) {
+	case 'y':
+	case 'Y':
+	case '1':
+		*res = true;
+		return 0;
+	case 'n':
+	case 'N':
+	case '0':
+		*res = false;
+		return 0;
+	default:
+		break;
+	}
+
+	return -EINVAL;
+}
+EXPORT_SYMBOL(kstrtobool);
+
 #define kstrto_from_user(f, g, type)					\
 int f(const char __user *s, size_t count, unsigned int base, type *res)	\
 {									\
@@ -345,3 +379,4 @@ kstrto_from_user(kstrtou16_from_user,	kstrtou16,	u16);
 kstrto_from_user(kstrtos16_from_user,	kstrtos16,	s16);
 kstrto_from_user(kstrtou8_from_user,	kstrtou8,	u8);
 kstrto_from_user(kstrtos8_from_user,	kstrtos8,	s8);
+kstrto_from_user(kstrtobool_from_user,	kstrtobool,	bool);
diff --git a/lib/string.c b/lib/string.c
index 0323c0d5629a..1a90db9bc6e1 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -630,35 +630,6 @@ bool sysfs_streq(const char *s1, const char *s2)
 }
 EXPORT_SYMBOL(sysfs_streq);
 
-/**
- * strtobool - convert common user inputs into boolean values
- * @s: input string
- * @res: result
- *
- * This routine returns 0 iff the first character is one of 'Yy1Nn0'.
- * Otherwise it will return -EINVAL.  Value pointed to by res is
- * updated upon finding a match.
- */
-int strtobool(const char *s, bool *res)
-{
-	switch (s[0]) {
-	case 'y':
-	case 'Y':
-	case '1':
-		*res = true;
-		break;
-	case 'n':
-	case 'N':
-	case '0':
-		*res = false;
-		break;
-	default:
-		return -EINVAL;
-	}
-	return 0;
-}
-EXPORT_SYMBOL(strtobool);
-
 #ifndef __HAVE_ARCH_MEMSET
 /**
  * memset - Fill a region of memory with the given value
-- 
2.6.3

[toc] | [next] | [standalone]

#1327264

FromAndy Shevchenko <andy.shevchenko@gmail.com>
Date2016-02-04 23:50 +0100
Message-ID<qYAST-8lI-43@gated-at.bofh.it>
In reply to#1327193
On Thu, Feb 4, 2016 at 11:00 PM, Kees Cook <keescook@chromium.org> wrote:
> Create the kstrtobool_from_user helper and moves strtobool logic into
> the new kstrtobool (matching all the other kstrto* functions). Provides
> an inline wrapper for existing strtobool callers.
>
> Signed-off-by: Kees Cook <keescook@chromium.org>

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

One minor below.

> ---
>  include/linux/kernel.h |  3 +++
>  include/linux/string.h |  6 +++++-
>  lib/kstrtox.c          | 35 +++++++++++++++++++++++++++++++++++
>  lib/string.c           | 29 -----------------------------
>  4 files changed, 43 insertions(+), 30 deletions(-)
>
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index f31638c6e873..cdc25f47a23f 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -357,6 +357,7 @@ int __must_check kstrtou16(const char *s, unsigned int base, u16 *res);
>  int __must_check kstrtos16(const char *s, unsigned int base, s16 *res);
>  int __must_check kstrtou8(const char *s, unsigned int base, u8 *res);
>  int __must_check kstrtos8(const char *s, unsigned int base, s8 *res);
> +int __must_check kstrtobool(const char *s, unsigned int base, bool *res);
>
>  int __must_check kstrtoull_from_user(const char __user *s, size_t count, unsigned int base, unsigned long long *res);
>  int __must_check kstrtoll_from_user(const char __user *s, size_t count, unsigned int base, long long *res);
> @@ -368,6 +369,8 @@ int __must_check kstrtou16_from_user(const char __user *s, size_t count, unsigne
>  int __must_check kstrtos16_from_user(const char __user *s, size_t count, unsigned int base, s16 *res);
>  int __must_check kstrtou8_from_user(const char __user *s, size_t count, unsigned int base, u8 *res);
>  int __must_check kstrtos8_from_user(const char __user *s, size_t count, unsigned int base, s8 *res);

> +int __must_check kstrtobool_from_user(const char __user *s, size_t count,
> +                                     unsigned int base, bool *res);

We already are using long lines here, perhaps do the same?

>
>  static inline int __must_check kstrtou64_from_user(const char __user *s, size_t count, unsigned int base, u64 *res)
>  {
> diff --git a/include/linux/string.h b/include/linux/string.h
> index 9eebc66d957a..d2fb21b1081d 100644
> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -128,7 +128,11 @@ 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 int strtobool(const char *s, bool *res);
> +extern int kstrtobool(const char *s, unsigned int base, bool *res);
> +static inline int strtobool(const char *s, bool *res)
> +{
> +       return kstrtobool(s, 0, res);
> +}
>
>  #ifdef CONFIG_BINARY_PRINTF
>  int vbin_printf(u32 *bin_buf, size_t size, const char *fmt, va_list args);
> diff --git a/lib/kstrtox.c b/lib/kstrtox.c
> index 94be244e8441..e18f088704d7 100644
> --- a/lib/kstrtox.c
> +++ b/lib/kstrtox.c
> @@ -321,6 +321,40 @@ int kstrtos8(const char *s, unsigned int base, s8 *res)
>  }
>  EXPORT_SYMBOL(kstrtos8);
>
> +/**
> + * kstrtobool - convert common user inputs into boolean values
> + * @s: input string
> + * @base: ignored
> + * @res: result
> + *
> + * This routine returns 0 iff the first character is one of 'Yy1Nn0'.
> + * Otherwise it will return -EINVAL.  Value pointed to by res is
> + * updated upon finding a match.
> + */
> +int kstrtobool(const char *s, unsigned int base, bool *res)
> +{
> +       if (!s)
> +               return -EINVAL;
> +
> +       switch (s[0]) {
> +       case 'y':
> +       case 'Y':
> +       case '1':
> +               *res = true;
> +               return 0;
> +       case 'n':
> +       case 'N':
> +       case '0':
> +               *res = false;
> +               return 0;
> +       default:
> +               break;
> +       }
> +
> +       return -EINVAL;
> +}
> +EXPORT_SYMBOL(kstrtobool);
> +
>  #define kstrto_from_user(f, g, type)                                   \
>  int f(const char __user *s, size_t count, unsigned int base, type *res)        \
>  {                                                                      \
> @@ -345,3 +379,4 @@ kstrto_from_user(kstrtou16_from_user,       kstrtou16,      u16);
>  kstrto_from_user(kstrtos16_from_user,  kstrtos16,      s16);
>  kstrto_from_user(kstrtou8_from_user,   kstrtou8,       u8);
>  kstrto_from_user(kstrtos8_from_user,   kstrtos8,       s8);
> +kstrto_from_user(kstrtobool_from_user, kstrtobool,     bool);
> diff --git a/lib/string.c b/lib/string.c
> index 0323c0d5629a..1a90db9bc6e1 100644
> --- a/lib/string.c
> +++ b/lib/string.c
> @@ -630,35 +630,6 @@ bool sysfs_streq(const char *s1, const char *s2)
>  }
>  EXPORT_SYMBOL(sysfs_streq);
>
> -/**
> - * strtobool - convert common user inputs into boolean values
> - * @s: input string
> - * @res: result
> - *
> - * This routine returns 0 iff the first character is one of 'Yy1Nn0'.
> - * Otherwise it will return -EINVAL.  Value pointed to by res is
> - * updated upon finding a match.
> - */
> -int strtobool(const char *s, bool *res)
> -{
> -       switch (s[0]) {
> -       case 'y':
> -       case 'Y':
> -       case '1':
> -               *res = true;
> -               break;
> -       case 'n':
> -       case 'N':
> -       case '0':
> -               *res = false;
> -               break;
> -       default:
> -               return -EINVAL;
> -       }
> -       return 0;
> -}
> -EXPORT_SYMBOL(strtobool);
> -
>  #ifndef __HAVE_ARCH_MEMSET
>  /**
>   * memset - Fill a region of memory with the given value
> --
> 2.6.3
>



-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [next] | [standalone]

#1327282

FromKees Cook <keescook@chromium.org>
Date2016-02-05 00:00 +0100
Message-ID<qYB2z-8qn-41@gated-at.bofh.it>
In reply to#1327264
On Thu, Feb 4, 2016 at 2:43 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Thu, Feb 4, 2016 at 11:00 PM, Kees Cook <keescook@chromium.org> wrote:
>> Create the kstrtobool_from_user helper and moves strtobool logic into
>> the new kstrtobool (matching all the other kstrto* functions). Provides
>> an inline wrapper for existing strtobool callers.
>>
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>
> One minor below.

Thanks!

>
>> ---
>>  include/linux/kernel.h |  3 +++
>>  include/linux/string.h |  6 +++++-
>>  lib/kstrtox.c          | 35 +++++++++++++++++++++++++++++++++++
>>  lib/string.c           | 29 -----------------------------
>>  4 files changed, 43 insertions(+), 30 deletions(-)
>>
>> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
>> index f31638c6e873..cdc25f47a23f 100644
>> --- a/include/linux/kernel.h
>> +++ b/include/linux/kernel.h
>> @@ -357,6 +357,7 @@ int __must_check kstrtou16(const char *s, unsigned int base, u16 *res);
>>  int __must_check kstrtos16(const char *s, unsigned int base, s16 *res);
>>  int __must_check kstrtou8(const char *s, unsigned int base, u8 *res);
>>  int __must_check kstrtos8(const char *s, unsigned int base, s8 *res);
>> +int __must_check kstrtobool(const char *s, unsigned int base, bool *res);
>>
>>  int __must_check kstrtoull_from_user(const char __user *s, size_t count, unsigned int base, unsigned long long *res);
>>  int __must_check kstrtoll_from_user(const char __user *s, size_t count, unsigned int base, long long *res);
>> @@ -368,6 +369,8 @@ int __must_check kstrtou16_from_user(const char __user *s, size_t count, unsigne
>>  int __must_check kstrtos16_from_user(const char __user *s, size_t count, unsigned int base, s16 *res);
>>  int __must_check kstrtou8_from_user(const char __user *s, size_t count, unsigned int base, u8 *res);
>>  int __must_check kstrtos8_from_user(const char __user *s, size_t count, unsigned int base, s8 *res);
>
>> +int __must_check kstrtobool_from_user(const char __user *s, size_t count,
>> +                                     unsigned int base, bool *res);
>
> We already are using long lines here, perhaps do the same?

I went back and forth on that, and decided that between checkpatch
yelling at me, and trying to be an agent of less entropy, I wrapped
the definition. I am fine either way, though.

-Kees

>
>>
>>  static inline int __must_check kstrtou64_from_user(const char __user *s, size_t count, unsigned int base, u64 *res)
>>  {
>> diff --git a/include/linux/string.h b/include/linux/string.h
>> index 9eebc66d957a..d2fb21b1081d 100644
>> --- a/include/linux/string.h
>> +++ b/include/linux/string.h
>> @@ -128,7 +128,11 @@ 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 int strtobool(const char *s, bool *res);
>> +extern int kstrtobool(const char *s, unsigned int base, bool *res);
>> +static inline int strtobool(const char *s, bool *res)
>> +{
>> +       return kstrtobool(s, 0, res);
>> +}
>>
>>  #ifdef CONFIG_BINARY_PRINTF
>>  int vbin_printf(u32 *bin_buf, size_t size, const char *fmt, va_list args);
>> diff --git a/lib/kstrtox.c b/lib/kstrtox.c
>> index 94be244e8441..e18f088704d7 100644
>> --- a/lib/kstrtox.c
>> +++ b/lib/kstrtox.c
>> @@ -321,6 +321,40 @@ int kstrtos8(const char *s, unsigned int base, s8 *res)
>>  }
>>  EXPORT_SYMBOL(kstrtos8);
>>
>> +/**
>> + * kstrtobool - convert common user inputs into boolean values
>> + * @s: input string
>> + * @base: ignored
>> + * @res: result
>> + *
>> + * This routine returns 0 iff the first character is one of 'Yy1Nn0'.
>> + * Otherwise it will return -EINVAL.  Value pointed to by res is
>> + * updated upon finding a match.
>> + */
>> +int kstrtobool(const char *s, unsigned int base, bool *res)
>> +{
>> +       if (!s)
>> +               return -EINVAL;
>> +
>> +       switch (s[0]) {
>> +       case 'y':
>> +       case 'Y':
>> +       case '1':
>> +               *res = true;
>> +               return 0;
>> +       case 'n':
>> +       case 'N':
>> +       case '0':
>> +               *res = false;
>> +               return 0;
>> +       default:
>> +               break;
>> +       }
>> +
>> +       return -EINVAL;
>> +}
>> +EXPORT_SYMBOL(kstrtobool);
>> +
>>  #define kstrto_from_user(f, g, type)                                   \
>>  int f(const char __user *s, size_t count, unsigned int base, type *res)        \
>>  {                                                                      \
>> @@ -345,3 +379,4 @@ kstrto_from_user(kstrtou16_from_user,       kstrtou16,      u16);
>>  kstrto_from_user(kstrtos16_from_user,  kstrtos16,      s16);
>>  kstrto_from_user(kstrtou8_from_user,   kstrtou8,       u8);
>>  kstrto_from_user(kstrtos8_from_user,   kstrtos8,       s8);
>> +kstrto_from_user(kstrtobool_from_user, kstrtobool,     bool);
>> diff --git a/lib/string.c b/lib/string.c
>> index 0323c0d5629a..1a90db9bc6e1 100644
>> --- a/lib/string.c
>> +++ b/lib/string.c
>> @@ -630,35 +630,6 @@ bool sysfs_streq(const char *s1, const char *s2)
>>  }
>>  EXPORT_SYMBOL(sysfs_streq);
>>
>> -/**
>> - * strtobool - convert common user inputs into boolean values
>> - * @s: input string
>> - * @res: result
>> - *
>> - * This routine returns 0 iff the first character is one of 'Yy1Nn0'.
>> - * Otherwise it will return -EINVAL.  Value pointed to by res is
>> - * updated upon finding a match.
>> - */
>> -int strtobool(const char *s, bool *res)
>> -{
>> -       switch (s[0]) {
>> -       case 'y':
>> -       case 'Y':
>> -       case '1':
>> -               *res = true;
>> -               break;
>> -       case 'n':
>> -       case 'N':
>> -       case '0':
>> -               *res = false;
>> -               break;
>> -       default:
>> -               return -EINVAL;
>> -       }
>> -       return 0;
>> -}
>> -EXPORT_SYMBOL(strtobool);
>> -
>>  #ifndef __HAVE_ARCH_MEMSET
>>  /**
>>   * memset - Fill a region of memory with the given value
>> --
>> 2.6.3
>>
>
>
>
> --
> With Best Regards,
> Andy Shevchenko



-- 
Kees Cook
Chrome OS & Brillo Security

[toc] | [prev] | [next] | [standalone]

#1327357

FromRasmus Villemoes <linux@rasmusvillemoes.dk>
Date2016-02-05 01:00 +0100
Message-ID<qYBYB-AO-3@gated-at.bofh.it>
In reply to#1327193
On Thu, Feb 04 2016, Kees Cook <keescook@chromium.org> wrote:

> Create the kstrtobool_from_user helper and moves strtobool logic into
> the new kstrtobool (matching all the other kstrto* functions). Provides
> an inline wrapper for existing strtobool callers.
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  include/linux/kernel.h |  3 +++
>  include/linux/string.h |  6 +++++-
>  lib/kstrtox.c          | 35 +++++++++++++++++++++++++++++++++++
>  lib/string.c           | 29 -----------------------------
>  4 files changed, 43 insertions(+), 30 deletions(-)
>
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index f31638c6e873..cdc25f47a23f 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -357,6 +357,7 @@ int __must_check kstrtou16(const char *s, unsigned int base, u16 *res);
>  int __must_check kstrtos16(const char *s, unsigned int base, s16 *res);
>  int __must_check kstrtou8(const char *s, unsigned int base, u8 *res);
>  int __must_check kstrtos8(const char *s, unsigned int base, s8 *res);
> +int __must_check kstrtobool(const char *s, unsigned int base, bool *res);
>  
>  int __must_check kstrtoull_from_user(const char __user *s, size_t count, unsigned int base, unsigned long long *res);
>  int __must_check kstrtoll_from_user(const char __user *s, size_t count, unsigned int base, long long *res);
> @@ -368,6 +369,8 @@ int __must_check kstrtou16_from_user(const char __user *s, size_t count, unsigne
>  int __must_check kstrtos16_from_user(const char __user *s, size_t count, unsigned int base, s16 *res);
>  int __must_check kstrtou8_from_user(const char __user *s, size_t count, unsigned int base, u8 *res);
>  int __must_check kstrtos8_from_user(const char __user *s, size_t count, unsigned int base, s8 *res);
> +int __must_check kstrtobool_from_user(const char __user *s, size_t count,
> +				      unsigned int base, bool *res);
>  
>  static inline int __must_check kstrtou64_from_user(const char __user *s, size_t count, unsigned int base, u64 *res)
>  {
> diff --git a/include/linux/string.h b/include/linux/string.h
> index 9eebc66d957a..d2fb21b1081d 100644
> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -128,7 +128,11 @@ 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 int strtobool(const char *s, bool *res);
> +extern int kstrtobool(const char *s, unsigned int base, bool *res);
> +static inline int strtobool(const char *s, bool *res)
> +{
> +	return kstrtobool(s, 0, res);
> +}
>  
>  #ifdef CONFIG_BINARY_PRINTF
>  int vbin_printf(u32 *bin_buf, size_t size, const char *fmt, va_list args);
> diff --git a/lib/kstrtox.c b/lib/kstrtox.c
> index 94be244e8441..e18f088704d7 100644
> --- a/lib/kstrtox.c
> +++ b/lib/kstrtox.c
> @@ -321,6 +321,40 @@ int kstrtos8(const char *s, unsigned int base, s8 *res)
>  }
>  EXPORT_SYMBOL(kstrtos8);
>  
> +/**
> + * kstrtobool - convert common user inputs into boolean values
> + * @s: input string
> + * @base: ignored
> + * @res: result
> + *
> + * This routine returns 0 iff the first character is one of 'Yy1Nn0'.
> + * Otherwise it will return -EINVAL.  Value pointed to by res is
> + * updated upon finding a match.
> + */
> +int kstrtobool(const char *s, unsigned int base, bool *res)
> +{

Being able to create the kstrtobool_from_user with a single macro
invocation is convenient, but I don't think that justifies the ugliness
of having an unused parameter. People reading this code or trying to use
the interface will wonder what it's doing there, and it will generate
slightly larger code for all the users of strtobool.

So I'd just make a separate explicit definition of kstrtobool_from_user
(the stack buffer sizing doesn't apply to the strings we want to parse
anyway, though 11 is of course plenty).

Rasmus

[toc] | [prev] | [next] | [standalone]

#1328066

FromKees Cook <keescook@chromium.org>
Date2016-02-05 22:00 +0100
Message-ID<qYVDY-5dV-1@gated-at.bofh.it>
In reply to#1327357
On Thu, Feb 4, 2016 at 3:55 PM, Rasmus Villemoes
<linux@rasmusvillemoes.dk> wrote:
> On Thu, Feb 04 2016, Kees Cook <keescook@chromium.org> wrote:
>
>> Create the kstrtobool_from_user helper and moves strtobool logic into
>> the new kstrtobool (matching all the other kstrto* functions). Provides
>> an inline wrapper for existing strtobool callers.
>>
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>> ---
>>  include/linux/kernel.h |  3 +++
>>  include/linux/string.h |  6 +++++-
>>  lib/kstrtox.c          | 35 +++++++++++++++++++++++++++++++++++
>>  lib/string.c           | 29 -----------------------------
>>  4 files changed, 43 insertions(+), 30 deletions(-)
>>
>> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
>> index f31638c6e873..cdc25f47a23f 100644
>> --- a/include/linux/kernel.h
>> +++ b/include/linux/kernel.h
>> @@ -357,6 +357,7 @@ int __must_check kstrtou16(const char *s, unsigned int base, u16 *res);
>>  int __must_check kstrtos16(const char *s, unsigned int base, s16 *res);
>>  int __must_check kstrtou8(const char *s, unsigned int base, u8 *res);
>>  int __must_check kstrtos8(const char *s, unsigned int base, s8 *res);
>> +int __must_check kstrtobool(const char *s, unsigned int base, bool *res);
>>
>>  int __must_check kstrtoull_from_user(const char __user *s, size_t count, unsigned int base, unsigned long long *res);
>>  int __must_check kstrtoll_from_user(const char __user *s, size_t count, unsigned int base, long long *res);
>> @@ -368,6 +369,8 @@ int __must_check kstrtou16_from_user(const char __user *s, size_t count, unsigne
>>  int __must_check kstrtos16_from_user(const char __user *s, size_t count, unsigned int base, s16 *res);
>>  int __must_check kstrtou8_from_user(const char __user *s, size_t count, unsigned int base, u8 *res);
>>  int __must_check kstrtos8_from_user(const char __user *s, size_t count, unsigned int base, s8 *res);
>> +int __must_check kstrtobool_from_user(const char __user *s, size_t count,
>> +                                   unsigned int base, bool *res);
>>
>>  static inline int __must_check kstrtou64_from_user(const char __user *s, size_t count, unsigned int base, u64 *res)
>>  {
>> diff --git a/include/linux/string.h b/include/linux/string.h
>> index 9eebc66d957a..d2fb21b1081d 100644
>> --- a/include/linux/string.h
>> +++ b/include/linux/string.h
>> @@ -128,7 +128,11 @@ 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 int strtobool(const char *s, bool *res);
>> +extern int kstrtobool(const char *s, unsigned int base, bool *res);
>> +static inline int strtobool(const char *s, bool *res)
>> +{
>> +     return kstrtobool(s, 0, res);
>> +}
>>
>>  #ifdef CONFIG_BINARY_PRINTF
>>  int vbin_printf(u32 *bin_buf, size_t size, const char *fmt, va_list args);
>> diff --git a/lib/kstrtox.c b/lib/kstrtox.c
>> index 94be244e8441..e18f088704d7 100644
>> --- a/lib/kstrtox.c
>> +++ b/lib/kstrtox.c
>> @@ -321,6 +321,40 @@ int kstrtos8(const char *s, unsigned int base, s8 *res)
>>  }
>>  EXPORT_SYMBOL(kstrtos8);
>>
>> +/**
>> + * kstrtobool - convert common user inputs into boolean values
>> + * @s: input string
>> + * @base: ignored
>> + * @res: result
>> + *
>> + * This routine returns 0 iff the first character is one of 'Yy1Nn0'.
>> + * Otherwise it will return -EINVAL.  Value pointed to by res is
>> + * updated upon finding a match.
>> + */
>> +int kstrtobool(const char *s, unsigned int base, bool *res)
>> +{
>
> Being able to create the kstrtobool_from_user with a single macro
> invocation is convenient, but I don't think that justifies the ugliness
> of having an unused parameter. People reading this code or trying to use
> the interface will wonder what it's doing there, and it will generate
> slightly larger code for all the users of strtobool.
>
> So I'd just make a separate explicit definition of kstrtobool_from_user
> (the stack buffer sizing doesn't apply to the strings we want to parse
> anyway, though 11 is of course plenty).

Okay, thanks. So many things were bothering me, but I feared code
duplication would be seen as worse. I'm much happier to drop the
unused argument. :)

I'll send a v3 with all the changes.

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web