Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1312426 > unrolled thread
| Started by | Kees Cook <keescook@chromium.org> |
|---|---|
| First post | 2016-01-19 19:20 +0100 |
| Last post | 2016-01-23 00:30 +0100 |
| Articles | 3 — 2 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.
[PATCH v4 2/8] lib: add "on" and "off" to strtobool Kees Cook <keescook@chromium.org> - 2016-01-19 19:20 +0100
Re: [PATCH v4 2/8] lib: add "on" and "off" to strtobool Joe Perches <joe@perches.com> - 2016-01-20 03:10 +0100
Re: [PATCH v4 2/8] lib: add "on" and "off" to strtobool Kees Cook <keescook@chromium.org> - 2016-01-23 00:30 +0100
| From | Kees Cook <keescook@chromium.org> |
|---|---|
| Date | 2016-01-19 19:20 +0100 |
| Subject | [PATCH v4 2/8] lib: add "on" and "off" to strtobool |
| Message-ID | <qSJ2O-36W-17@gated-at.bofh.it> |
Several places in the kernel expect to use "on" and "off" for their
boolean signifiers, so add them to strtobool.
Signed-off-by: Kees Cook <keescook@chromium.org>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Daniel Borkmann <daniel@iogearbox.net>
---
lib/string.c | 24 +++++++++++++++++++++---
1 file changed, 21 insertions(+), 3 deletions(-)
diff --git a/lib/string.c b/lib/string.c
index 0323c0d5629a..091570708db7 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -635,12 +635,15 @@ EXPORT_SYMBOL(sysfs_streq);
* @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.
+ * This routine returns 0 iff the first character is one of 'Yy1Nn0', or
+ * [oO][NnFf] for "on" and "off". Otherwise it will return -EINVAL. Value
+ * pointed to by res is updated upon finding a match.
*/
int strtobool(const char *s, bool *res)
{
+ if (!s)
+ return -EINVAL;
+
switch (s[0]) {
case 'y':
case 'Y':
@@ -652,6 +655,21 @@ int strtobool(const char *s, bool *res)
case '0':
*res = false;
break;
+ case 'o':
+ case 'O':
+ switch (s[1]) {
+ case 'n':
+ case 'N':
+ *res = true;
+ break;
+ case 'f':
+ case 'F':
+ *res = false;
+ break;
+ default:
+ return -EINVAL;
+ }
+ break;
default:
return -EINVAL;
}
--
2.6.3
[toc] | [next] | [standalone]
| From | Joe Perches <joe@perches.com> |
|---|---|
| Date | 2016-01-20 03:10 +0100 |
| Message-ID | <qSQnF-8bo-49@gated-at.bofh.it> |
| In reply to | #1312426 |
On Tue, 2016-01-19 at 10:08 -0800, Kees Cook wrote:
> Several places in the kernel expect to use "on" and "off" for their
> boolean signifiers, so add them to strtobool.
Several places in the kernel use a char address like
fs/cifs/cifs_debug.c
char c;
...
if (strtobool(&c, ...))
Using s[1] might cause problems for those uses.
> diff --git a/lib/string.c b/lib/string.c
[]
> @@ -635,12 +635,15 @@ EXPORT_SYMBOL(sysfs_streq);
> * @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.
> + * This routine returns 0 iff the first character is one of 'Yy1Nn0', or
> + * [oO][NnFf] for "on" and "off". Otherwise it will return -EINVAL. Value
> + * pointed to by res is updated upon finding a match.
> */
> int strtobool(const char *s, bool *res)
> {
> + if (!s)
> + return -EINVAL;
> +
> switch (s[0]) {
> case 'y':
> case 'Y':
> @@ -652,6 +655,21 @@ int strtobool(const char *s, bool *res)
> case '0':
> *res = false;
> break;
> + case 'o':
> + case 'O':
> + switch (s[1]) {
> + case 'n':
> + case 'N':
> + *res = true;
> + break;
> + case 'f':
> + case 'F':
Perhaps
switch (tolower(s[1])) {
is more readable
> + *res = false;
> + break;
> + default:
> + return -EINVAL;
> + }
> + break;
or maybe /* fallthrough */
> default:
> return -EINVAL;
> }
[toc] | [prev] | [next] | [standalone]
| From | Kees Cook <keescook@chromium.org> |
|---|---|
| Date | 2016-01-23 00:30 +0100 |
| Message-ID | <qTTjs-2Bp-11@gated-at.bofh.it> |
| In reply to | #1312817 |
On Tue, Jan 19, 2016 at 6:09 PM, Joe Perches <joe@perches.com> wrote:
> On Tue, 2016-01-19 at 10:08 -0800, Kees Cook wrote:
>> Several places in the kernel expect to use "on" and "off" for their
>> boolean signifiers, so add them to strtobool.
>
> Several places in the kernel use a char address like
> fs/cifs/cifs_debug.c
>
>
> char c;
> ...
>
>
> if (strtobool(&c, ...))
>
> Using s[1] might cause problems for those uses.
Oh ew. Thanks for noticing that.
>> diff --git a/lib/string.c b/lib/string.c
> []
>> @@ -635,12 +635,15 @@ EXPORT_SYMBOL(sysfs_streq);
>> * @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.
>> + * This routine returns 0 iff the first character is one of 'Yy1Nn0', or
>> + * [oO][NnFf] for "on" and "off". Otherwise it will return -EINVAL. Value
>> + * pointed to by res is updated upon finding a match.
>> */
>> int strtobool(const char *s, bool *res)
>> {
>> + if (!s)
>> + return -EINVAL;
>> +
>> switch (s[0]) {
>> case 'y':
>> case 'Y':
>> @@ -652,6 +655,21 @@ int strtobool(const char *s, bool *res)
>> case '0':
>> *res = false;
>> break;
>> + case 'o':
>> + case 'O':
>> + switch (s[1]) {
>> + case 'n':
>> + case 'N':
>> + *res = true;
>> + break;
>> + case 'f':
>> + case 'F':
>
> Perhaps
> switch (tolower(s[1])) {
> is more readable
I opted to let the compiler deal with optimizing this, and I left the
switch statement as close to original as possible.
-Kees
>
>> + *res = false;
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> + break;
>
> or maybe /* fallthrough */
>
>> default:
>> return -EINVAL;
>> }
>
--
Kees Cook
Chrome OS & Brillo Security
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web