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, );
> 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, );
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, , end))
-
