Re: [PATCH 6/9] Eliminate unused ESCAPE_NULL, ESCAPE_SPACE flags
On Thu, Sep 05, 2019 at 03:44:30PM -0400, J. Bruce Fields wrote: > From: "J. Bruce Fields" > > I can see how some finer-grained flags could be useful, but for now I'm > trying to simplify, so let's just remove these unused ones. > > Note the trickiest part is actually the tests, and I still need to check > them. Currently this _tries_ to follow the shorthand character classes which is established by tools. For example, "\s" = "[ \t\r\n\f]". Also it (almost?) matches the counterpart, i.e. string_unescape() classes. So, if we would need something else, perhaps better to do it in the separate flags. -- With Best Regards, Andy Shevchenko
Re: [PATCH 6/9] Eliminate unused ESCAPE_NULL, ESCAPE_SPACE flags
On Thu, Sep 05, 2019 at 03:44:30PM -0400, J. Bruce Fields wrote: > From: "J. Bruce Fields" > > I can see how some finer-grained flags could be useful, but for now I'm > trying to simplify, so let's just remove these unused ones. I might change the Subject to "merge ESCAPE_{NULL,SPACE,SPECIAL}" or so. > Note the trickiest part is actually the tests, and I still need to check > them. It looks correct to me, though I haven't run them myself yet. One thing to add might be adding a NULL test with explicit calls to string_escape_mem()? > > Signed-off-by: J. Bruce Fields Acked-by: Kees Cook -Kees > --- > fs/proc/array.c| 2 +- > include/linux/string_helpers.h | 6 ++-- > lib/string_helpers.c | 54 +++ > lib/test-string_helpers.c | 58 -- > 4 files changed, 14 insertions(+), 106 deletions(-) > > diff --git a/fs/proc/array.c b/fs/proc/array.c > index 46dcb6f0eccf..982c95d09176 100644 > --- a/fs/proc/array.c > +++ b/fs/proc/array.c > @@ -111,7 +111,7 @@ void proc_task_name(struct seq_file *m, struct > task_struct *p, bool escape) > size = seq_get_buf(m, &buf); > if (escape) { > ret = string_escape_str(tcomm, buf, size, > - ESCAPE_SPACE | ESCAPE_SPECIAL, "\n\\"); > + ESCAPE_SPECIAL, "\n\\"); > if (ret >= size) > ret = -1; > } else { > diff --git a/include/linux/string_helpers.h b/include/linux/string_helpers.h > index 8a299a29b767..7bf0d137373d 100644 > --- a/include/linux/string_helpers.h > +++ b/include/linux/string_helpers.h > @@ -41,15 +41,13 @@ static inline int string_unescape_any_inplace(char *buf) > return string_unescape_any(buf, buf, 0); > } > > -#define ESCAPE_SPACE 0x01 > #define ESCAPE_SPECIAL 0x02 > -#define ESCAPE_NULL 0x04 > #define ESCAPE_OCTAL 0x08 > -#define ESCAPE_ANY \ > - (ESCAPE_SPACE | ESCAPE_OCTAL | ESCAPE_SPECIAL | ESCAPE_NULL) > +#define ESCAPE_ANY (ESCAPE_OCTAL | ESCAPE_SPECIAL) > #define ESCAPE_NP0x10 > #define ESCAPE_ANY_NP(ESCAPE_ANY | ESCAPE_NP) > #define ESCAPE_HEX 0x20 > +#define ESCAPE_FLAG_MASK 0x3a > > int string_escape_mem(const char *src, size_t isz, char *dst, size_t osz, > unsigned int flags, const char *only); > diff --git a/lib/string_helpers.c b/lib/string_helpers.c > index 963050c0283e..ac72159d3980 100644 > --- a/lib/string_helpers.c > +++ b/lib/string_helpers.c > @@ -310,7 +310,7 @@ static bool escape_passthrough(unsigned char c, char > **dst, char *end) > return true; > } > > -static bool escape_space(unsigned char c, char **dst, char *end) > +static bool escape_special(unsigned char c, char **dst, char *end) > { > char *out = *dst; > unsigned char to; > @@ -331,27 +331,6 @@ static bool escape_space(unsigned char c, char **dst, > char *end) > case '\f': > to = 'f'; > break; > - default: > - return false; > - } > - > - if (out < end) > - *out = '\\'; > - ++out; > - if (out < end) > - *out = to; > - ++out; > - > - *dst = out; > - return true; > -} > - > -static bool escape_special(unsigned char c, char **dst, char *end) > -{ > - char *out = *dst; > - unsigned char to; > - > - switch (c) { > case '\\': > to = '\\'; > break; > @@ -361,6 +340,9 @@ static bool escape_special(unsigned char c, char **dst, > char *end) > case '\e': > to = 'e'; > break; > + case '\0': > + to = '0'; > + break; > default: > return false; > } > @@ -376,24 +358,6 @@ static bool escape_special(unsigned char c, char **dst, > char *end) > return true; > } > > -static bool escape_null(unsigned char c, char **dst, char *end) > -{ > - char *out = *dst; > - > - if (c) > - return false; > - > - if (out < end) > - *out = '\\'; > - ++out; > - if (out < end) > - *out = '0'; > - ++out; > - > - *dst = out; > - return true; > -} > - > static bool escape_octal(unsigned char c, char **dst, char *end) > { > char *out = *dst; > @@ -465,17 +429,15 @@ static bool escape_hex(unsigned char c, char **dst, > char *end) > * destination buffer will not be NULL-terminated, thus caller have to append > * it if needs. The supported flags are:: > * > - * %ESCAPE_SPACE: (special white space, not space itself) > + * %ESCAPE_SPECIAL: > * '\f' - form feed > * '\n' - new line > * '\r' - carriage return > * '\t' - horizontal tab > * '\v' - vertical tab > - * %ESCAPE_SPECIAL: > * '\\' - backslash > * '\a' - alert (BEL) >
[PATCH 6/9] Eliminate unused ESCAPE_NULL, ESCAPE_SPACE flags
From: "J. Bruce Fields" I can see how some finer-grained flags could be useful, but for now I'm trying to simplify, so let's just remove these unused ones. Note the trickiest part is actually the tests, and I still need to check them. Signed-off-by: J. Bruce Fields --- fs/proc/array.c| 2 +- include/linux/string_helpers.h | 6 ++-- lib/string_helpers.c | 54 +++ lib/test-string_helpers.c | 58 -- 4 files changed, 14 insertions(+), 106 deletions(-) diff --git a/fs/proc/array.c b/fs/proc/array.c index 46dcb6f0eccf..982c95d09176 100644 --- a/fs/proc/array.c +++ b/fs/proc/array.c @@ -111,7 +111,7 @@ void proc_task_name(struct seq_file *m, struct task_struct *p, bool escape) size = seq_get_buf(m, &buf); if (escape) { ret = string_escape_str(tcomm, buf, size, - ESCAPE_SPACE | ESCAPE_SPECIAL, "\n\\"); + ESCAPE_SPECIAL, "\n\\"); if (ret >= size) ret = -1; } else { diff --git a/include/linux/string_helpers.h b/include/linux/string_helpers.h index 8a299a29b767..7bf0d137373d 100644 --- a/include/linux/string_helpers.h +++ b/include/linux/string_helpers.h @@ -41,15 +41,13 @@ static inline int string_unescape_any_inplace(char *buf) return string_unescape_any(buf, buf, 0); } -#define ESCAPE_SPACE 0x01 #define ESCAPE_SPECIAL 0x02 -#define ESCAPE_NULL0x04 #define ESCAPE_OCTAL 0x08 -#define ESCAPE_ANY \ - (ESCAPE_SPACE | ESCAPE_OCTAL | ESCAPE_SPECIAL | ESCAPE_NULL) +#define ESCAPE_ANY (ESCAPE_OCTAL | ESCAPE_SPECIAL) #define ESCAPE_NP 0x10 #define ESCAPE_ANY_NP (ESCAPE_ANY | ESCAPE_NP) #define ESCAPE_HEX 0x20 +#define ESCAPE_FLAG_MASK 0x3a int string_escape_mem(const char *src, size_t isz, char *dst, size_t osz, unsigned int flags, const char *only); diff --git a/lib/string_helpers.c b/lib/string_helpers.c index 963050c0283e..ac72159d3980 100644 --- a/lib/string_helpers.c +++ b/lib/string_helpers.c @@ -310,7 +310,7 @@ static bool escape_passthrough(unsigned char c, char **dst, char *end) return true; } -static bool escape_space(unsigned char c, char **dst, char *end) +static bool escape_special(unsigned char c, char **dst, char *end) { char *out = *dst; unsigned char to; @@ -331,27 +331,6 @@ static bool escape_space(unsigned char c, char **dst, char *end) case '\f': to = 'f'; break; - default: - return false; - } - - if (out < end) - *out = '\\'; - ++out; - if (out < end) - *out = to; - ++out; - - *dst = out; - return true; -} - -static bool escape_special(unsigned char c, char **dst, char *end) -{ - char *out = *dst; - unsigned char to; - - switch (c) { case '\\': to = '\\'; break; @@ -361,6 +340,9 @@ static bool escape_special(unsigned char c, char **dst, char *end) case '\e': to = 'e'; break; + case '\0': + to = '0'; + break; default: return false; } @@ -376,24 +358,6 @@ static bool escape_special(unsigned char c, char **dst, char *end) return true; } -static bool escape_null(unsigned char c, char **dst, char *end) -{ - char *out = *dst; - - if (c) - return false; - - if (out < end) - *out = '\\'; - ++out; - if (out < end) - *out = '0'; - ++out; - - *dst = out; - return true; -} - static bool escape_octal(unsigned char c, char **dst, char *end) { char *out = *dst; @@ -465,17 +429,15 @@ static bool escape_hex(unsigned char c, char **dst, char *end) * destination buffer will not be NULL-terminated, thus caller have to append * it if needs. The supported flags are:: * - * %ESCAPE_SPACE: (special white space, not space itself) + * %ESCAPE_SPECIAL: * '\f' - form feed * '\n' - new line * '\r' - carriage return * '\t' - horizontal tab * '\v' - vertical tab - * %ESCAPE_SPECIAL: * '\\' - backslash * '\a' - alert (BEL) * '\e' - escape - * %ESCAPE_NULL: * '\0' - null * %ESCAPE_OCTAL: * '\NNN' - byte with octal value NNN (3 digits) @@ -519,15 +481,9 @@ int string_escape_mem(const char *src, size_t isz, char *dst, size_t osz, (is_dict && !strchr(only, c))) { /* do nothing */ } else { - if (flags & ESCAPE_SPACE && escape_space(c, &p, end)) -