linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] lib / string_helpers: introduce string_escape_mem
@ 2014-07-02 13:20 Andy Shevchenko
  2014-07-02 13:20 ` [PATCH 1/6] lib / string_helpers: clean up test suite Andy Shevchenko
                   ` (5 more replies)
  0 siblings, 6 replies; 17+ messages in thread
From: Andy Shevchenko @ 2014-07-02 13:20 UTC (permalink / raw)
  To: John W. Linville, Johannes Berg, devel, linux-wireless,
	linux-kernel, Greg Kroah-Hartman, Andrew Morton
  Cc: Andy Shevchenko

The introduced function is a kind of opposite to string_unescape. We have
several users of such functionality each of them created custom implementation.
The series contains clean up of test suite, adding new call, and switching few
users to use it.

Test suite covers all of existing and most of potential use cases.

Andy Shevchenko (6):
  lib / string_helpers: clean up test suite
  lib / string_helpers: introduce string_escape_mem()
  lib80211: re-use string_escape_mem_any_np()
  staging: wlan-ng: re-use string_escape_mem()
  staging: rtl8192e: re-use string_escape_mem()
  staging: rtl8192u: re-use string_escape_mem()

 drivers/staging/rtl8192e/rtllib.h              |  13 +-
 drivers/staging/rtl8192u/ieee80211/ieee80211.h |  13 +-
 drivers/staging/wlan-ng/prism2sta.c            |  29 +--
 include/linux/string_helpers.h                 |  83 ++++++++
 lib/string_helpers.c                           | 215 +++++++++++++++++++++
 lib/test-string_helpers.c                      | 250 +++++++++++++++++++++++--
 net/wireless/lib80211.c                        |  24 +--
 7 files changed, 551 insertions(+), 76 deletions(-)

-- 
2.0.0


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/6] lib / string_helpers: clean up test suite
  2014-07-02 13:20 [PATCH 0/6] lib / string_helpers: introduce string_escape_mem Andy Shevchenko
@ 2014-07-02 13:20 ` Andy Shevchenko
  2014-07-02 21:54   ` Andrew Morton
  2014-07-02 13:20 ` [PATCH 2/6] lib / string_helpers: introduce string_escape_mem() Andy Shevchenko
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 17+ messages in thread
From: Andy Shevchenko @ 2014-07-02 13:20 UTC (permalink / raw)
  To: John W. Linville, Johannes Berg, devel, linux-wireless,
	linux-kernel, Greg Kroah-Hartman, Andrew Morton
  Cc: Andy Shevchenko

This patch prepares test suite for a following update. It introduces
test_string_check_buf() helper which checks the result and dumps an error.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 lib/test-string_helpers.c | 39 +++++++++++++++++++++++++++------------
 1 file changed, 27 insertions(+), 12 deletions(-)

diff --git a/lib/test-string_helpers.c b/lib/test-string_helpers.c
index 6ac48de..ea86e02 100644
--- a/lib/test-string_helpers.c
+++ b/lib/test-string_helpers.c
@@ -10,6 +10,26 @@
 #include <linux/string.h>
 #include <linux/string_helpers.h>
 
+static __init bool test_string_check_buf(const char *name, unsigned int flags,
+					 char *in, size_t p,
+					 char *out_real, size_t q_real,
+					 char *out_test, size_t q_test)
+{
+	if (q_real == q_test && !memcmp(out_test, out_real, q_test))
+		return true;
+
+	pr_err("Test '%s' failed: flags = %u\n", name, flags);
+
+	print_hex_dump(KERN_WARNING, "Input: ", DUMP_PREFIX_NONE, 16, 1,
+		       in, p, true);
+	print_hex_dump(KERN_WARNING, "Expected: ", DUMP_PREFIX_NONE, 16, 1,
+		       out_test, q_test, true);
+	print_hex_dump(KERN_WARNING, "Got: ", DUMP_PREFIX_NONE, 16, 1,
+		       out_real, q_real, true);
+
+	return false;
+}
+
 struct test_string {
 	const char *in;
 	const char *out;
@@ -39,7 +59,8 @@ static const struct test_string strings[] __initconst = {
 	},
 };
 
-static void __init test_string_unescape(unsigned int flags, bool inplace)
+static void __init test_string_unescape(const char *name, unsigned int flags,
+					bool inplace)
 {
 	char in[256];
 	char out_test[256];
@@ -77,15 +98,8 @@ static void __init test_string_unescape(unsigned int flags, bool inplace)
 		q_real = string_unescape(in, out_real, q_real, flags);
 	}
 
-	if (q_real != q_test || memcmp(out_test, out_real, q_test)) {
-		pr_warn("Test failed: flags = %u\n", flags);
-		print_hex_dump(KERN_WARNING, "Input: ",
-			       DUMP_PREFIX_NONE, 16, 1, in, p - 1, true);
-		print_hex_dump(KERN_WARNING, "Expected: ",
-			       DUMP_PREFIX_NONE, 16, 1, out_test, q_test, true);
-		print_hex_dump(KERN_WARNING, "Got: ",
-			       DUMP_PREFIX_NONE, 16, 1, out_real, q_real, true);
-	}
+	test_string_check_buf(name, flags, in, p - 1, out_real, q_real,
+			      out_test, q_test);
 }
 
 static int __init test_string_helpers_init(void)
@@ -94,8 +108,9 @@ static int __init test_string_helpers_init(void)
 
 	pr_info("Running tests...\n");
 	for (i = 0; i < UNESCAPE_ANY + 1; i++)
-		test_string_unescape(i, false);
-	test_string_unescape(get_random_int() % (UNESCAPE_ANY + 1), true);
+		test_string_unescape("unescape", i, false);
+	test_string_unescape("unescape inplace",
+			     get_random_int() % (UNESCAPE_ANY + 1), true);
 
 	return -EINVAL;
 }
-- 
2.0.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2/6] lib / string_helpers: introduce string_escape_mem()
  2014-07-02 13:20 [PATCH 0/6] lib / string_helpers: introduce string_escape_mem Andy Shevchenko
  2014-07-02 13:20 ` [PATCH 1/6] lib / string_helpers: clean up test suite Andy Shevchenko
@ 2014-07-02 13:20 ` Andy Shevchenko
  2014-07-02 22:01   ` Andrew Morton
  2014-07-02 13:20 ` [PATCH 3/6] lib80211: re-use string_escape_mem_any_np() Andy Shevchenko
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 17+ messages in thread
From: Andy Shevchenko @ 2014-07-02 13:20 UTC (permalink / raw)
  To: John W. Linville, Johannes Berg, devel, linux-wireless,
	linux-kernel, Greg Kroah-Hartman, Andrew Morton
  Cc: Andy Shevchenko

This is almost the opposite function to string_unescape(). Nevertheless it
handles \0 and could be used for any byte buffer.

The documentation is supplied together with the function prototype.

The test cases covers most of the scenarios and would be expanded later on.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/string_helpers.h |  83 ++++++++++++++++
 lib/string_helpers.c           | 215 +++++++++++++++++++++++++++++++++++++++++
 lib/test-string_helpers.c      | 213 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 511 insertions(+)

diff --git a/include/linux/string_helpers.h b/include/linux/string_helpers.h
index 3eeee96..db13f20 100644
--- a/include/linux/string_helpers.h
+++ b/include/linux/string_helpers.h
@@ -71,4 +71,87 @@ 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_NP		0x10
+#define ESCAPE_ANY_NP		(ESCAPE_ANY | ESCAPE_NP)
+#define ESCAPE_HEX		0x20
+
+/**
+ * string_escape_mem - quote characters in the given memory buffer
+ * @src:	source buffer (unescaped)
+ * @isz:	source buffer size
+ * @dst:	destination buffer (escaped)
+ * @osz:	destination buffer size
+ * @flags:	combination of the flags (bitwise OR):
+ *	%ESCAPE_SPACE:
+ *		'\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)
+ *	%ESCAPE_ANY:
+ *		all previous together
+ *	%ESCAPE_NP:
+ *		escape only non-printable characters (checked by isprint)
+ *	%ESCAPE_ANY_NP:
+ *		all previous together
+ *	%ESCAPE_HEX:
+ *		'\xHH' - byte with hexadecimal value HH (2 digits)
+ * @esc:	NULL-terminated string of characters any of which, if found in
+ *		the source, has to be escaped
+ *
+ * Description:
+ * The process of escaping byte buffer includes several parts. They are applied
+ * in the following sequence.
+ *	1. The character is matched to the printable class, if asked, and in
+ *	   case of match it passes through to the output.
+ *	2. The character is not matched to the one from @esc string and thus
+ *	   must go as is to the output.
+ *	3. The character is checked if it falls into the class given by @flags.
+ *	   %ESCAPE_OCTAL and %ESCAPE_HEX are going last since they cover any
+ *	   character. Note that they actually can't go together, otherwise
+ *	   %ESCAPE_HEX will be ignored.
+ *
+ * Caller must provide valid source and destination pointers. Be aware that
+ * destination buffer will not be NULL-terminated, thus caller have to append
+ * it if needs.
+ *
+ * Return: amount of characters processed to the destination buffer, or
+ * 	   %-ENOMEM if the size of buffer is not enough to put an escaped
+ * 	   character.
+ */
+int string_escape_mem(const char *src, size_t isz, char *dst, size_t osz,
+		unsigned int flags, const char *esc);
+
+static inline int string_escape_mem_any_np(const char *src, size_t isz,
+		char *dst, size_t osz, const char *esc)
+{
+	return string_escape_mem(src, isz, dst, osz, ESCAPE_ANY_NP, esc);
+}
+
+static inline int string_escape_str(const char *src, char *dst, size_t sz,
+		unsigned int flags, const char *esc)
+{
+	return string_escape_mem(src, strlen(src), dst, sz, flags, esc);
+}
+
+static inline int string_escape_str_any_np(const char *src, char *dst,
+		size_t sz, const char *esc)
+{
+	return string_escape_str(src, dst, sz, ESCAPE_ANY_NP, esc);
+}
+
 #endif
diff --git a/lib/string_helpers.c b/lib/string_helpers.c
index 29033f3..00112eb 100644
--- a/lib/string_helpers.c
+++ b/lib/string_helpers.c
@@ -8,6 +8,8 @@
 #include <linux/math64.h>
 #include <linux/export.h>
 #include <linux/ctype.h>
+#include <linux/errno.h>
+#include <linux/string.h>
 #include <linux/string_helpers.h>
 
 /**
@@ -202,3 +204,216 @@ int string_unescape(char *src, char *dst, size_t size, unsigned int flags)
 	return out - dst;
 }
 EXPORT_SYMBOL(string_unescape);
+
+static int escape_passthrough(unsigned char c, char **dst, size_t *osz)
+{
+	char *out = *dst;
+
+	if (*osz < 1)
+		return -ENOMEM;
+
+	*out++ = c;
+
+	*dst = out;
+	*osz -= 1;
+
+	return 1;
+}
+
+static int escape_space(unsigned char c, char **dst, size_t *osz)
+{
+	char *out = *dst;
+	unsigned char to;
+
+	if (*osz < 2)
+		return -ENOMEM;
+
+	switch (c) {
+	case '\n':
+		to = 'n';
+		break;
+	case '\r':
+		to = 'r';
+		break;
+	case '\t':
+		to = 't';
+		break;
+	case '\v':
+		to = 'v';
+		break;
+	case '\f':
+		to = 'f';
+		break;
+	default:
+		return 0;
+	}
+
+	*out++ = '\\';
+	*out++ = to;
+
+	*dst = out;
+	*osz -= 2;
+
+	return 1;
+}
+
+static int escape_special(unsigned char c, char **dst, size_t *osz)
+{
+	char *out = *dst;
+	unsigned char to;
+
+	if (*osz < 2)
+		return -ENOMEM;
+
+	switch (c) {
+	case '\\':
+		to = '\\';
+		break;
+	case '\a':
+		to = 'a';
+		break;
+	case '\e':
+		to = 'e';
+		break;
+	default:
+		return 0;
+	}
+
+	*out++ = '\\';
+	*out++ = to;
+
+	*dst = out;
+	*osz -= 2;
+
+	return 1;
+}
+
+static int escape_null(unsigned char c, char **dst, size_t *osz)
+{
+	char *out = *dst;
+
+	if (*osz < 2)
+		return -ENOMEM;
+
+	if (c)
+		return 0;
+
+	*out++ = '\\';
+	*out++ = '0';
+
+	*dst = out;
+	*osz -= 2;
+
+	return 1;
+}
+
+static int escape_octal(unsigned char c, char **dst, size_t *osz)
+{
+	char *out = *dst;
+
+	if (*osz < 4)
+		return -ENOMEM;
+
+	*out++ = '\\';
+	*out++ = ((c >> 6) & 0x07) + '0';
+	*out++ = ((c >> 3) & 0x07) + '0';
+	*out++ = ((c >> 0) & 0x07) + '0';
+
+	*dst = out;
+	*osz -= 4;
+
+	return 1;
+}
+
+static int escape_hex(unsigned char c, char **dst, size_t *osz)
+{
+	char *out = *dst;
+
+	if (*osz < 4)
+		return -ENOMEM;
+
+	*out++ = '\\';
+	*out++ = 'x';
+	*out++ = hex_asc_hi(c);
+	*out++ = hex_asc_lo(c);
+
+	*dst = out;
+	*osz -= 4;
+
+	return 1;
+}
+
+int string_escape_mem(const char *src, size_t isz, char *dst, size_t osz,
+		      unsigned int flags, const char *esc)
+{
+	char *out = dst;
+	int ret = 0;
+
+	while (isz--) {
+		unsigned char c = *src++;
+
+		/*
+		 * Apply rules in the following sequence:
+		 *	- the character is printable, when @flags has
+		 *	  %ESCAPE_NP bit set
+		 *	- the @esc string is supplied and does not contain a
+		 *	  character under question
+		 *	- the character doesn't fall into a class of symbols
+		 *	  defined by given @flags
+		 * In these cases we just pass through a character to the
+		 * output buffer.
+		 */
+		if ((flags & ESCAPE_NP && isprint(c)) ||
+		    (esc && *esc && !strchr(esc, c))) {
+			/* do nothing */
+		} else {
+			if (flags & ESCAPE_SPACE) {
+				ret = escape_space(c, &out, &osz);
+				if (ret < 0)
+					break;
+				if (ret > 0)
+					continue;
+			}
+
+			if (flags & ESCAPE_SPECIAL) {
+				ret = escape_special(c, &out, &osz);
+				if (ret < 0)
+					break;
+				if (ret > 0)
+					continue;
+			}
+
+			if (flags & ESCAPE_NULL) {
+				ret = escape_null(c, &out, &osz);
+				if (ret < 0)
+					break;
+				if (ret > 0)
+					continue;
+			}
+
+			/* ESCAPE_OCTAL and ESCAPE_HEX always go last */
+			if (flags & ESCAPE_OCTAL) {
+				ret = escape_octal(c, &out, &osz);
+				if (ret < 0)
+					break;
+				continue;
+			}
+			if (flags & ESCAPE_HEX) {
+				ret = escape_hex(c, &out, &osz);
+				if (ret < 0)
+					break;
+				continue;
+			}
+		}
+
+		ret = escape_passthrough(c, &out, &osz);
+		if (ret < 0)
+			break;
+	}
+
+	if (ret < 0)
+		return ret;
+
+	return out - dst;
+}
+EXPORT_SYMBOL(string_escape_mem);
diff --git a/lib/test-string_helpers.c b/lib/test-string_helpers.c
index ea86e02..4b022ae 100644
--- a/lib/test-string_helpers.c
+++ b/lib/test-string_helpers.c
@@ -102,6 +102,209 @@ static void __init test_string_unescape(const char *name, unsigned int flags,
 			      out_test, q_test);
 }
 
+struct test_string_1 {
+	const char *out;
+	unsigned int flags;
+};
+
+#define	TEST_STRING_2_MAX_S1		32
+struct test_string_2 {
+	const char *in;
+	struct test_string_1 s1[TEST_STRING_2_MAX_S1];
+};
+
+#define	TEST_STRING_2_DICT_0		NULL
+static const struct test_string_2 escape0[] __initconst = {{
+	.in = "\f\\ \n\r\t\v",
+	.s1 = {{
+		.out = "\\f\\ \\n\\r\\t\\v",
+		.flags = ESCAPE_SPACE,
+	},{
+		.out = "\\f\\134\\040\\n\\r\\t\\v",
+		.flags = ESCAPE_SPACE | ESCAPE_OCTAL,
+	},{
+		.out = "\\f\\x5c\\x20\\n\\r\\t\\v",
+		.flags = ESCAPE_SPACE | ESCAPE_HEX,
+	},{
+		/* terminator */
+	}},
+},{
+	.in = "\\h\\\"\a\e\\",
+	.s1 = {{
+		.out = "\\\\h\\\\\"\\a\\e\\\\",
+		.flags = ESCAPE_SPECIAL,
+	},{
+		.out = "\\\\\\150\\\\\\042\\a\\e\\\\",
+		.flags = ESCAPE_SPECIAL | ESCAPE_OCTAL,
+	},{
+		.out = "\\\\\\x68\\\\\\x22\\a\\e\\\\",
+		.flags = ESCAPE_SPECIAL | ESCAPE_HEX,
+	},{
+		/* terminator */
+	}},
+},{
+	.in = "\eb \\C\007\"\x90\r]",
+	.s1 = {{
+		.out = "\eb \\C\007\"\x90\\r]",
+		.flags = ESCAPE_SPACE,
+	},{
+		.out = "\\eb \\\\C\\a\"\x90\r]",
+		.flags = ESCAPE_SPECIAL,
+	},{
+		.out = "\\eb \\\\C\\a\"\x90\\r]",
+		.flags = ESCAPE_SPACE | ESCAPE_SPECIAL,
+	},{
+		.out = "\\033\\142\\040\\134\\103\\007\\042\\220\\015\\135",
+		.flags = ESCAPE_OCTAL,
+	},{
+		.out = "\\033\\142\\040\\134\\103\\007\\042\\220\\r\\135",
+		.flags = ESCAPE_SPACE | ESCAPE_OCTAL,
+	},{
+		.out = "\\e\\142\\040\\\\\\103\\a\\042\\220\\015\\135",
+		.flags = ESCAPE_SPECIAL | ESCAPE_OCTAL,
+	},{
+		.out = "\\e\\142\\040\\\\\\103\\a\\042\\220\\r\\135",
+		.flags = ESCAPE_SPACE | ESCAPE_SPECIAL | ESCAPE_OCTAL,
+	},{
+		.out = "\eb \\C\007\"\x90\r]",
+		.flags = ESCAPE_NP,
+	},{
+		.out = "\eb \\C\007\"\x90\\r]",
+		.flags = ESCAPE_SPACE | ESCAPE_NP,
+	},{
+		.out = "\\eb \\C\\a\"\x90\r]",
+		.flags = ESCAPE_SPECIAL | ESCAPE_NP,
+	},{
+		.out = "\\eb \\C\\a\"\x90\\r]",
+		.flags = ESCAPE_SPACE | ESCAPE_SPECIAL | ESCAPE_NP,
+	},{
+		.out = "\\033b \\C\\007\"\\220\\015]",
+		.flags = ESCAPE_OCTAL | ESCAPE_NP,
+	},{
+		.out = "\\033b \\C\\007\"\\220\\r]",
+		.flags = ESCAPE_SPACE | ESCAPE_OCTAL | ESCAPE_NP,
+	},{
+		.out = "\\eb \\C\\a\"\\220\\r]",
+		.flags = ESCAPE_SPECIAL | ESCAPE_SPACE | ESCAPE_OCTAL |
+			 ESCAPE_NP,
+	},{
+		.out = "\\x1bb \\C\\x07\"\\x90\\x0d]",
+		.flags = ESCAPE_NP | ESCAPE_HEX,
+	},{
+		/* terminator */
+	}},
+},{
+	/* terminator */
+}};
+
+#define	TEST_STRING_2_DICT_1		"b\\ \t\r"
+static const struct test_string_2 escape1[] __initconst = {{
+	.in = "\f\\ \n\r\t\v",
+	.s1 = {{
+		.out = "\f\\134\\040\n\\015\\011\v",
+		.flags = ESCAPE_OCTAL,
+	},{
+		.out = "\f\\x5c\\x20\n\\x0d\\x09\v",
+		.flags = ESCAPE_HEX,
+	},{
+		/* terminator */
+	}},
+},{
+	.in = "\\h\\\"\a\e\\",
+	.s1 = {{
+		.out = "\\134h\\134\"\a\e\\134",
+		.flags = ESCAPE_OCTAL,
+	},{
+		/* terminator */
+	}},
+},{
+	.in = "\eb \\C\007\"\x90\r]",
+	.s1 = {{
+		.out = "\e\\142\\040\\134C\007\"\x90\\015]",
+		.flags = ESCAPE_OCTAL,
+	},{
+		/* terminator */
+	}},
+},{
+	/* terminator */
+}};
+
+static __init const char *test_string_find_match(const struct test_string_2 *s2,
+						 unsigned int flags)
+{
+	const struct test_string_1 *s1 = s2->s1;
+	unsigned int i;
+
+	if (!flags)
+		return s2->in;
+
+	/* Test cases are NULL-aware */
+	flags &= ~ESCAPE_NULL;
+
+	/* ESCAPE_OCTAL has a higher priority */
+	if (flags & ESCAPE_OCTAL)
+		flags &= ~ESCAPE_HEX;
+
+	for (i = 0; i < TEST_STRING_2_MAX_S1 && s1->out; i++, s1++)
+		if (s1->flags == flags)
+			return s1->out;
+	return NULL;
+}
+
+static __init void test_string_escape(const char *name,
+				      const struct test_string_2 *s2,
+				      unsigned int flags, const char *esc)
+{
+	char in[256];
+	char out_test[512];
+	char out_real[512];
+	int p = 0, q_test = 0, q_real = sizeof(out_real);
+
+	for (; s2->in; s2++) {
+		const char *out;
+		int len;
+
+		/* NULL injection */
+		if (flags & ESCAPE_NULL) {
+			in[p++] = '\0';
+			out_test[q_test++] = '\\';
+			out_test[q_test++] = '0';
+		}
+
+		/* Don't try strings that have no output */
+		out = test_string_find_match(s2, flags);
+		if (!out)
+			continue;
+
+		/* Copy string to in buffer */
+		len = strlen(s2->in);
+		memcpy(&in[p], s2->in, len);
+		p += len;
+
+		/* Copy expected result for given flags */
+		len = strlen(out);
+		memcpy(&out_test[q_test], out, len);
+		q_test += len;
+	}
+
+	q_real = string_escape_mem(in, p, out_real, q_real, flags, esc);
+
+	test_string_check_buf(name, flags, in, p, out_real, q_real, out_test, q_test);
+}
+
+static __init void test_string_escape_nomem(void)
+{
+	char *in = "\eb \\C\007\"\x90\r]";
+	char out[64];
+	int rc = -ENOMEM, ret;
+
+	ret = string_escape_str_any_np(in, out, strlen(in), NULL);
+	if (ret == rc)
+		return;
+
+	pr_err("Test 'escape nomem' failed: got %d instead of %d\n", ret, rc);
+}
+
 static int __init test_string_helpers_init(void)
 {
 	unsigned int i;
@@ -112,6 +315,16 @@ static int __init test_string_helpers_init(void)
 	test_string_unescape("unescape inplace",
 			     get_random_int() % (UNESCAPE_ANY + 1), true);
 
+	/* Without dictionary */
+	for (i = 0; i < (ESCAPE_ANY_NP | ESCAPE_HEX) + 1; i++)
+		test_string_escape("escape 0", escape0, i, TEST_STRING_2_DICT_0);
+
+	/* With dictionary */
+	for (i = 0; i < (ESCAPE_ANY_NP | ESCAPE_HEX) + 1; i++)
+		test_string_escape("escape 1", escape1, i, TEST_STRING_2_DICT_1);
+
+	test_string_escape_nomem();
+
 	return -EINVAL;
 }
 module_init(test_string_helpers_init);
-- 
2.0.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 3/6] lib80211: re-use string_escape_mem_any_np()
  2014-07-02 13:20 [PATCH 0/6] lib / string_helpers: introduce string_escape_mem Andy Shevchenko
  2014-07-02 13:20 ` [PATCH 1/6] lib / string_helpers: clean up test suite Andy Shevchenko
  2014-07-02 13:20 ` [PATCH 2/6] lib / string_helpers: introduce string_escape_mem() Andy Shevchenko
@ 2014-07-02 13:20 ` Andy Shevchenko
  2014-07-02 13:43   ` Joe Perches
  2014-07-02 13:20 ` [PATCH 4/6] staging: wlan-ng: re-use string_escape_mem() Andy Shevchenko
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 17+ messages in thread
From: Andy Shevchenko @ 2014-07-02 13:20 UTC (permalink / raw)
  To: John W. Linville, Johannes Berg, devel, linux-wireless,
	linux-kernel, Greg Kroah-Hartman, Andrew Morton
  Cc: Andy Shevchenko

In kernel we have function to escape a given string. Let's use it instead of
custom approach.

This fixes a bug. The current implementation wrongly prints octal numbers: only
two first digits are used in case when 3 are required and the rest of the
string ends up cut off.

Additionally the \f, \v, \a, and \e are escaped to their alphabetic
representation. It's safe to do since the print_ssid() is currently used for
messaging only.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 net/wireless/lib80211.c | 24 ++----------------------
 1 file changed, 2 insertions(+), 22 deletions(-)

diff --git a/net/wireless/lib80211.c b/net/wireless/lib80211.c
index a55c27b..0c2f67b 100644
--- a/net/wireless/lib80211.c
+++ b/net/wireless/lib80211.c
@@ -22,6 +22,7 @@
 #include <linux/init.h>
 #include <linux/slab.h>
 #include <linux/string.h>
+#include <linux/string_helpers.h>
 
 #include <net/lib80211.h>
 
@@ -48,31 +49,10 @@ static void lib80211_crypt_deinit_handler(unsigned long data);
 
 const char *print_ssid(char *buf, const char *ssid, u8 ssid_len)
 {
-	const char *s = ssid;
 	char *d = buf;
 
 	ssid_len = min_t(u8, ssid_len, IEEE80211_MAX_SSID_LEN);
-	while (ssid_len--) {
-		if (isprint(*s)) {
-			*d++ = *s++;
-			continue;
-		}
-
-		*d++ = '\\';
-		if (*s == '\0')
-			*d++ = '0';
-		else if (*s == '\n')
-			*d++ = 'n';
-		else if (*s == '\r')
-			*d++ = 'r';
-		else if (*s == '\t')
-			*d++ = 't';
-		else if (*s == '\\')
-			*d++ = '\\';
-		else
-			d += snprintf(d, 3, "%03o", *s);
-		s++;
-	}
+	d += string_escape_mem_any_np(ssid, ssid_len, buf, ~0UL, NULL);
 	*d = '\0';
 	return buf;
 }
-- 
2.0.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 4/6] staging: wlan-ng: re-use string_escape_mem()
  2014-07-02 13:20 [PATCH 0/6] lib / string_helpers: introduce string_escape_mem Andy Shevchenko
                   ` (2 preceding siblings ...)
  2014-07-02 13:20 ` [PATCH 3/6] lib80211: re-use string_escape_mem_any_np() Andy Shevchenko
@ 2014-07-02 13:20 ` Andy Shevchenko
  2014-07-02 13:20 ` [PATCH 5/6] staging: rtl8192e: " Andy Shevchenko
  2014-07-02 13:20 ` [PATCH 6/6] staging: rtl8192u: " Andy Shevchenko
  5 siblings, 0 replies; 17+ messages in thread
From: Andy Shevchenko @ 2014-07-02 13:20 UTC (permalink / raw)
  To: John W. Linville, Johannes Berg, devel, linux-wireless,
	linux-kernel, Greg Kroah-Hartman, Andrew Morton
  Cc: Andy Shevchenko

This is a generic function to escape strings by given criteria. Let's use it
instead of custom approach.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/staging/wlan-ng/prism2sta.c | 29 ++++++-----------------------
 1 file changed, 6 insertions(+), 23 deletions(-)

diff --git a/drivers/staging/wlan-ng/prism2sta.c b/drivers/staging/wlan-ng/prism2sta.c
index 209e4db..1e44c09 100644
--- a/drivers/staging/wlan-ng/prism2sta.c
+++ b/drivers/staging/wlan-ng/prism2sta.c
@@ -61,6 +61,7 @@
 #include <linux/workqueue.h>
 #include <linux/byteorder/generic.h>
 #include <linux/ctype.h>
+#include <linux/string_helpers.h>
 
 #include <linux/io.h>
 #include <linux/delay.h>
@@ -81,27 +82,6 @@
 #include "hfa384x.h"
 #include "prism2mgmt.h"
 
-/* Create a string of printable chars from something that might not be */
-/* It's recommended that the str be 4*len + 1 bytes long */
-#define wlan_mkprintstr(buf, buflen, str, strlen) \
-{ \
-	int i = 0; \
-	int j = 0; \
-	memset(str, 0, (strlen)); \
-	for (i = 0; i < (buflen); i++) { \
-		if (isprint((buf)[i])) { \
-			(str)[j] = (buf)[i]; \
-			j++; \
-		} else { \
-			(str)[j] = '\\'; \
-			(str)[j+1] = 'x'; \
-			(str)[j+2] = hex_asc_hi((buf)[i]); \
-			(str)[j+3] = hex_asc_lo((buf)[i]); \
-			j += 4; \
-		} \
-	} \
-}
-
 static char *dev_info = "prism2_usb";
 static wlandevice_t *create_wlan(void);
 
@@ -862,8 +842,11 @@ static int prism2sta_getcardinfo(wlandevice_t *wlandev)
 	result = hfa384x_drvr_getconfig(hw, HFA384x_RID_NICSERIALNUMBER,
 					snum, HFA384x_RID_NICSERIALNUMBER_LEN);
 	if (!result) {
-		wlan_mkprintstr(snum, HFA384x_RID_NICSERIALNUMBER_LEN,
-				pstr, sizeof(pstr));
+		result = string_escape_mem(snum,
+				HFA384x_RID_NICSERIALNUMBER_LEN,
+				pstr, sizeof(pstr), ESCAPE_HEX | ESCAPE_NP,
+				NULL);
+		pstr[result] = '\0';
 		netdev_info(wlandev->netdev, "Prism2 card SN: %s\n", pstr);
 	} else {
 		netdev_err(wlandev->netdev, "Failed to retrieve Prism2 Card SN\n");
-- 
2.0.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 5/6] staging: rtl8192e: re-use string_escape_mem()
  2014-07-02 13:20 [PATCH 0/6] lib / string_helpers: introduce string_escape_mem Andy Shevchenko
                   ` (3 preceding siblings ...)
  2014-07-02 13:20 ` [PATCH 4/6] staging: wlan-ng: re-use string_escape_mem() Andy Shevchenko
@ 2014-07-02 13:20 ` Andy Shevchenko
  2014-07-02 13:35   ` Joe Perches
  2014-07-02 13:20 ` [PATCH 6/6] staging: rtl8192u: " Andy Shevchenko
  5 siblings, 1 reply; 17+ messages in thread
From: Andy Shevchenko @ 2014-07-02 13:20 UTC (permalink / raw)
  To: John W. Linville, Johannes Berg, devel, linux-wireless,
	linux-kernel, Greg Kroah-Hartman, Andrew Morton
  Cc: Andy Shevchenko

Let's use kernel's library function to escape a buffer.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/staging/rtl8192e/rtllib.h | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/rtl8192e/rtllib.h b/drivers/staging/rtl8192e/rtllib.h
index 83f5f57..cb99160 100644
--- a/drivers/staging/rtl8192e/rtllib.h
+++ b/drivers/staging/rtl8192e/rtllib.h
@@ -31,6 +31,7 @@
 #include <linux/timer.h>
 #include <linux/sched.h>
 #include <linux/semaphore.h>
+#include <linux/string_helpers.h>
 
 #include <linux/delay.h>
 #include <linux/wireless.h>
@@ -2956,7 +2957,6 @@ extern inline int rtllib_get_scans(struct rtllib_device *ieee)
 static inline const char *escape_essid(const char *essid, u8 essid_len)
 {
 	static char escaped[IW_ESSID_MAX_SIZE * 2 + 1];
-	const char *s = essid;
 	char *d = escaped;
 
 	if (rtllib_is_empty_essid(essid, essid_len)) {
@@ -2965,15 +2965,8 @@ static inline const char *escape_essid(const char *essid, u8 essid_len)
 	}
 
 	essid_len = min(essid_len, (u8)IW_ESSID_MAX_SIZE);
-	while (essid_len--) {
-		if (*s == '\0') {
-			*d++ = '\\';
-			*d++ = '0';
-			s++;
-		} else {
-			*d++ = *s++;
-		}
-	}
+	d += string_escape_mem(essid, essid_len, escaped, sizeof(escaped) - 1,
+			       ESCAPE_NULL, NULL);
 	*d = '\0';
 	return escaped;
 }
-- 
2.0.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 6/6] staging: rtl8192u: re-use string_escape_mem()
  2014-07-02 13:20 [PATCH 0/6] lib / string_helpers: introduce string_escape_mem Andy Shevchenko
                   ` (4 preceding siblings ...)
  2014-07-02 13:20 ` [PATCH 5/6] staging: rtl8192e: " Andy Shevchenko
@ 2014-07-02 13:20 ` Andy Shevchenko
  5 siblings, 0 replies; 17+ messages in thread
From: Andy Shevchenko @ 2014-07-02 13:20 UTC (permalink / raw)
  To: John W. Linville, Johannes Berg, devel, linux-wireless,
	linux-kernel, Greg Kroah-Hartman, Andrew Morton
  Cc: Andy Shevchenko

Let's use kernel's library function to escape a buffer instead of custom code.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/staging/rtl8192u/ieee80211/ieee80211.h | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211.h b/drivers/staging/rtl8192u/ieee80211/ieee80211.h
index 1040bab..bb5f604 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211.h
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211.h
@@ -31,6 +31,7 @@
 #include <linux/sched.h>
 #include <linux/semaphore.h>
 #include <linux/interrupt.h>
+#include <linux/string_helpers.h>
 
 #include <linux/delay.h>
 #include <linux/wireless.h>
@@ -2576,7 +2577,6 @@ static inline int ieee80211_get_scans(struct ieee80211_device *ieee)
 
 static inline const char *escape_essid(const char *essid, u8 essid_len) {
 	static char escaped[IW_ESSID_MAX_SIZE * 2 + 1];
-	const char *s = essid;
 	char *d = escaped;
 
 	if (ieee80211_is_empty_essid(essid, essid_len)) {
@@ -2585,15 +2585,8 @@ static inline const char *escape_essid(const char *essid, u8 essid_len) {
 	}
 
 	essid_len = min(essid_len, (u8)IW_ESSID_MAX_SIZE);
-	while (essid_len--) {
-		if (*s == '\0') {
-			*d++ = '\\';
-			*d++ = '0';
-			s++;
-		} else {
-			*d++ = *s++;
-		}
-	}
+	d += string_escape_mem(essid, essid_len, escaped, sizeof(escaped) - 1,
+			       ESCAPE_NULL, NULL);
 	*d = '\0';
 	return escaped;
 }
-- 
2.0.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 5/6] staging: rtl8192e: re-use string_escape_mem()
  2014-07-02 13:20 ` [PATCH 5/6] staging: rtl8192e: " Andy Shevchenko
@ 2014-07-02 13:35   ` Joe Perches
  2014-07-02 14:11     ` Andy Shevchenko
  0 siblings, 1 reply; 17+ messages in thread
From: Joe Perches @ 2014-07-02 13:35 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: John W. Linville, Johannes Berg, devel, linux-wireless,
	linux-kernel, Greg Kroah-Hartman, Andrew Morton

On Wed, 2014-07-02 at 16:20 +0300, Andy Shevchenko wrote:
> Let's use kernel's library function to escape a buffer.
[]
> diff --git a/drivers/staging/rtl8192e/rtllib.h b/drivers/staging/rtl8192e/rtllib.h
[]
> @@ -2956,7 +2957,6 @@ extern inline int rtllib_get_scans(struct rtllib_device *ieee)
>  static inline const char *escape_essid(const char *essid, u8 essid_len)
>  {
>  	static char escaped[IW_ESSID_MAX_SIZE * 2 + 1];
> -	const char *s = essid;
>  	char *d = escaped;
>  
>  	if (rtllib_is_empty_essid(essid, essid_len)) {
> @@ -2965,15 +2965,8 @@ static inline const char *escape_essid(const char *essid, u8 essid_len)
>  	}
>  
>  	essid_len = min(essid_len, (u8)IW_ESSID_MAX_SIZE);
> -	while (essid_len--) {
> -		if (*s == '\0') {
> -			*d++ = '\\';
> -			*d++ = '0';
> -			s++;
> -		} else {
> -			*d++ = *s++;
> -		}
> -	}
> +	d += string_escape_mem(essid, essid_len, escaped, sizeof(escaped) - 1,
> +			       ESCAPE_NULL, NULL);

I'd've probably used

	d += string_escape_mem(essid, essid_len, d, ...
or
	d = escaped + string_escap_mem(essid, essid_len, escaped, ...

so there's some relation between the thing being added to

>  	*d = '\0';

or maybe not used d at all with

	escaped[1 + string_escape_mem(etc...)] = 0;

>  	return escaped;
>  }

Unrelated but this isn't a thread safe or multiple instance
safe function.

It seems it's used only in debugging message output though.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/6] lib80211: re-use string_escape_mem_any_np()
  2014-07-02 13:20 ` [PATCH 3/6] lib80211: re-use string_escape_mem_any_np() Andy Shevchenko
@ 2014-07-02 13:43   ` Joe Perches
  2014-07-02 14:06     ` Andy Shevchenko
  0 siblings, 1 reply; 17+ messages in thread
From: Joe Perches @ 2014-07-02 13:43 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: John W. Linville, Johannes Berg, devel, linux-wireless,
	linux-kernel, Greg Kroah-Hartman, Andrew Morton

On Wed, 2014-07-02 at 16:20 +0300, Andy Shevchenko wrote:
> In kernel we have function to escape a given string. Let's use it instead of
> custom approach.
> 
> This fixes a bug. The current implementation wrongly prints octal numbers: only
> two first digits are used in case when 3 are required and the rest of the
> string ends up cut off.
[]
> diff --git a/net/wireless/lib80211.c b/net/wireless/lib80211.c
[]
> @@ -48,31 +49,10 @@ static void lib80211_crypt_deinit_handler(unsigned long data);
>  
>  const char *print_ssid(char *buf, const char *ssid, u8 ssid_len)
>  {
> -	const char *s = ssid;
>  	char *d = buf;
>  
>  	ssid_len = min_t(u8, ssid_len, IEEE80211_MAX_SSID_LEN);
> -	while (ssid_len--) {
> -		if (isprint(*s)) {
> -			*d++ = *s++;
> -			continue;
> -		}
> -
> -		*d++ = '\\';
> -		if (*s == '\0')
> -			*d++ = '0';
> -		else if (*s == '\n')
> -			*d++ = 'n';
> -		else if (*s == '\r')
> -			*d++ = 'r';
> -		else if (*s == '\t')
> -			*d++ = 't';
> -		else if (*s == '\\')
> -			*d++ = '\\';
> -		else
> -			d += snprintf(d, 3, "%03o", *s);
> -		s++;
> -	}
> +	d += string_escape_mem_any_np(ssid, ssid_len, buf, ~0UL, NULL);
>  	*d = '\0';
>  	return buf;
>  }

This code looks like it was adapted from the old print_mac
ethernet code that was eventually replaced by a vsprintf
pointer extension %pM

So a better way to do this might be to add and use yet
another vsprintf %p<foo> extension for ssids.



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/6] lib80211: re-use string_escape_mem_any_np()
  2014-07-02 13:43   ` Joe Perches
@ 2014-07-02 14:06     ` Andy Shevchenko
  2014-07-02 16:30       ` Joe Perches
  0 siblings, 1 reply; 17+ messages in thread
From: Andy Shevchenko @ 2014-07-02 14:06 UTC (permalink / raw)
  To: Joe Perches
  Cc: John W. Linville, Johannes Berg, devel, linux-wireless,
	linux-kernel, Greg Kroah-Hartman, Andrew Morton

On Wed, 2014-07-02 at 06:43 -0700, Joe Perches wrote:
> On Wed, 2014-07-02 at 16:20 +0300, Andy Shevchenko wrote:
> > In kernel we have function to escape a given string. Let's use it instead of
> > custom approach.
> > 
> > This fixes a bug. The current implementation wrongly prints octal numbers: only
> > two first digits are used in case when 3 are required and the rest of the
> > string ends up cut off.

[]

> This code looks like it was adapted from the old print_mac
> ethernet code that was eventually replaced by a vsprintf
> pointer extension %pM
> 
> So a better way to do this might be to add and use yet
> another vsprintf %p<foo> extension for ssids.

Might be, but it
 - doesn't reduce necessity of string_escape_mem (not only ssid are
escaped in kernel)
 - prevents user to choose a rule what exactly their would like to
escape (look at the other patches against ssid escaping)

-- 
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 5/6] staging: rtl8192e: re-use string_escape_mem()
  2014-07-02 13:35   ` Joe Perches
@ 2014-07-02 14:11     ` Andy Shevchenko
  0 siblings, 0 replies; 17+ messages in thread
From: Andy Shevchenko @ 2014-07-02 14:11 UTC (permalink / raw)
  To: Joe Perches
  Cc: John W. Linville, Johannes Berg, devel, linux-wireless,
	linux-kernel, Greg Kroah-Hartman, Andrew Morton

On Wed, 2014-07-02 at 06:35 -0700, Joe Perches wrote:
> On Wed, 2014-07-02 at 16:20 +0300, Andy Shevchenko wrote:
> > Let's use kernel's library function to escape a buffer.

[]

> > @@ -2965,15 +2965,8 @@ static inline const char *escape_essid(const char *essid, u8 essid_len)
> >  	}
> >  
> >  	essid_len = min(essid_len, (u8)IW_ESSID_MAX_SIZE);
> > -	while (essid_len--) {
> > -		if (*s == '\0') {
> > -			*d++ = '\\';
> > -			*d++ = '0';
> > -			s++;
> > -		} else {
> > -			*d++ = *s++;
> > -		}
> > -	}
> > +	d += string_escape_mem(essid, essid_len, escaped, sizeof(escaped) - 1,
> > +			       ESCAPE_NULL, NULL);
> 
> I'd've probably used
> 
> 	d += string_escape_mem(essid, essid_len, d, ...
> or
> 	d = escaped + string_escap_mem(essid, essid_len, escaped, ...
> 
> so there's some relation between the thing being added to
> 
> >  	*d = '\0';
> 
> or maybe not used d at all with
> 
> 	escaped[1 + string_escape_mem(etc...)] = 0;

Perhaps without '1 + ' part. I could update this as well if someone
insists.

> 
> >  	return escaped;
> >  }
> 
> Unrelated but this isn't a thread safe or multiple instance
> safe function.

Do you mean escape_ssid() or string_escape_mem() or both?

For the string_escape_mem() I think caller should take care of.

> It seems it's used only in debugging message output though.


-- 
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/6] lib80211: re-use string_escape_mem_any_np()
  2014-07-02 14:06     ` Andy Shevchenko
@ 2014-07-02 16:30       ` Joe Perches
  2014-07-03 10:49         ` Andy Shevchenko
  0 siblings, 1 reply; 17+ messages in thread
From: Joe Perches @ 2014-07-02 16:30 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: John W. Linville, Johannes Berg, devel, linux-wireless,
	linux-kernel, Greg Kroah-Hartman, Andrew Morton

On Wed, 2014-07-02 at 17:06 +0300, Andy Shevchenko wrote:
> On Wed, 2014-07-02 at 06:43 -0700, Joe Perches wrote:
> > On Wed, 2014-07-02 at 16:20 +0300, Andy Shevchenko wrote:
> > > In kernel we have function to escape a given string. Let's use it instead of
> > > custom approach.
> > > 
> > > This fixes a bug. The current implementation wrongly prints octal numbers: only
> > > two first digits are used in case when 3 are required and the rest of the
> > > string ends up cut off.
> 
> []
> 
> > This code looks like it was adapted from the old print_mac
> > ethernet code that was eventually replaced by a vsprintf
> > pointer extension %pM
> > 
> > So a better way to do this might be to add and use yet
> > another vsprintf %p<foo> extension for ssids.
> 
> Might be, but it
>  - doesn't reduce necessity of string_escape_mem (not only ssid are
> escaped in kernel)
>  - prevents user to choose a rule what exactly their would like to
> escape (look at the other patches against ssid escaping)
> 

%pE<FLAGS> would allow the same arbitrary combinations
of your flags.



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/6] lib / string_helpers: clean up test suite
  2014-07-02 13:20 ` [PATCH 1/6] lib / string_helpers: clean up test suite Andy Shevchenko
@ 2014-07-02 21:54   ` Andrew Morton
  2014-07-03  9:34     ` Andy Shevchenko
  0 siblings, 1 reply; 17+ messages in thread
From: Andrew Morton @ 2014-07-02 21:54 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: John W. Linville, Johannes Berg, devel, linux-wireless,
	linux-kernel, Greg Kroah-Hartman

On Wed,  2 Jul 2014 16:20:24 +0300 Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> This patch prepares test suite for a following update. It introduces
> test_string_check_buf() helper which checks the result and dumps an error.
> 
> ...
>
> --- a/lib/test-string_helpers.c
> +++ b/lib/test-string_helpers.c
> @@ -10,6 +10,26 @@
>  #include <linux/string.h>
>  #include <linux/string_helpers.h>
>  
> +static __init bool test_string_check_buf(const char *name, unsigned int flags,
> +					 char *in, size_t p,
> +					 char *out_real, size_t q_real,
> +					 char *out_test, size_t q_test)
> +{
> +	if (q_real == q_test && !memcmp(out_test, out_real, q_test))
> +		return true;
> +
> +	pr_err("Test '%s' failed: flags = %u\n", name, flags);
> +
> +	print_hex_dump(KERN_WARNING, "Input: ", DUMP_PREFIX_NONE, 16, 1,
> +		       in, p, true);
> +	print_hex_dump(KERN_WARNING, "Expected: ", DUMP_PREFIX_NONE, 16, 1,
> +		       out_test, q_test, true);
> +	print_hex_dump(KERN_WARNING, "Got: ", DUMP_PREFIX_NONE, 16, 1,
> +		       out_real, q_real, true);

Seems strange to mix KERN_ERR and KERN_WARNING.  The code's always been
that way, but maybe it can be improved.



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/6] lib / string_helpers: introduce string_escape_mem()
  2014-07-02 13:20 ` [PATCH 2/6] lib / string_helpers: introduce string_escape_mem() Andy Shevchenko
@ 2014-07-02 22:01   ` Andrew Morton
  2014-07-03  9:53     ` Andy Shevchenko
  0 siblings, 1 reply; 17+ messages in thread
From: Andrew Morton @ 2014-07-02 22:01 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: John W. Linville, Johannes Berg, devel, linux-wireless,
	linux-kernel, Greg Kroah-Hartman

On Wed,  2 Jul 2014 16:20:25 +0300 Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> This is almost the opposite function to string_unescape(). Nevertheless it
> handles \0 and could be used for any byte buffer.
> 
> The documentation is supplied together with the function prototype.
> 
> The test cases covers most of the scenarios and would be expanded later on.
> 
> ...
>
> --- a/include/linux/string_helpers.h
> +++ b/include/linux/string_helpers.h
> @@ -71,4 +71,87 @@ 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_NP		0x10
> +#define ESCAPE_ANY_NP		(ESCAPE_ANY | ESCAPE_NP)
> +#define ESCAPE_HEX		0x20
> +
> +/**
> + * string_escape_mem - quote characters in the given memory buffer

It drive me nuts when the kerneldoc is in the .h file.  Who thinks of
looking there?  I realise that string_unescape() already did that, but
I'd prefer that we fix string_unescape() rather than imitate it.

> --- a/lib/string_helpers.c
> +++ b/lib/string_helpers.c

This is a lot of code!  Adds nearly a kbyte.  I'm surprised that
escaping a string is so verbose.

I wonder if the implementation really needs to be so comprehensive?

Would a table-driven approach be more compact?

>  static int __init test_string_helpers_init(void)
>  {
>  	unsigned int i;
> @@ -112,6 +315,16 @@ static int __init test_string_helpers_init(void)
>  	test_string_unescape("unescape inplace",
>  			     get_random_int() % (UNESCAPE_ANY + 1), true);
>  
> +	/* Without dictionary */
> +	for (i = 0; i < (ESCAPE_ANY_NP | ESCAPE_HEX) + 1; i++)
> +		test_string_escape("escape 0", escape0, i, TEST_STRING_2_DICT_0);
> +
> +	/* With dictionary */
> +	for (i = 0; i < (ESCAPE_ANY_NP | ESCAPE_HEX) + 1; i++)
> +		test_string_escape("escape 1", escape1, i, TEST_STRING_2_DICT_1);
> +
> +	test_string_escape_nomem();
> +
>  	return -EINVAL;
>  }

I wonder why this returns -EINVAL.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/6] lib / string_helpers: clean up test suite
  2014-07-02 21:54   ` Andrew Morton
@ 2014-07-03  9:34     ` Andy Shevchenko
  0 siblings, 0 replies; 17+ messages in thread
From: Andy Shevchenko @ 2014-07-03  9:34 UTC (permalink / raw)
  To: Andrew Morton
  Cc: John W. Linville, Johannes Berg, devel, linux-wireless,
	linux-kernel, Greg Kroah-Hartman

On Wed, 2014-07-02 at 14:54 -0700, Andrew Morton wrote:
> On Wed,  2 Jul 2014 16:20:24 +0300 Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

[]

> > +	pr_err("Test '%s' failed: flags = %u\n", name, flags);
> > +
> > +	print_hex_dump(KERN_WARNING, "Input: ", DUMP_PREFIX_NONE, 16, 1,
> > +		       in, p, true);
> > +	print_hex_dump(KERN_WARNING, "Expected: ", DUMP_PREFIX_NONE, 16, 1,
> > +		       out_test, q_test, true);
> > +	print_hex_dump(KERN_WARNING, "Got: ", DUMP_PREFIX_NONE, 16, 1,
> > +		       out_real, q_real, true);
> 
> Seems strange to mix KERN_ERR and KERN_WARNING.  The code's always been
> that way, but maybe it can be improved.

Will fix this.


-- 
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/6] lib / string_helpers: introduce string_escape_mem()
  2014-07-02 22:01   ` Andrew Morton
@ 2014-07-03  9:53     ` Andy Shevchenko
  0 siblings, 0 replies; 17+ messages in thread
From: Andy Shevchenko @ 2014-07-03  9:53 UTC (permalink / raw)
  To: Andrew Morton
  Cc: John W. Linville, Johannes Berg, devel, linux-wireless,
	linux-kernel, Greg Kroah-Hartman

On Wed, 2014-07-02 at 15:01 -0700, Andrew Morton wrote:
> On Wed,  2 Jul 2014 16:20:25 +0300 Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> 
> > This is almost the opposite function to string_unescape(). Nevertheless it
> > handles \0 and could be used for any byte buffer.
> > 
> > The documentation is supplied together with the function prototype.
> > 
> > The test cases covers most of the scenarios and would be expanded later on.
> > 
> > ...
> >
> > --- a/include/linux/string_helpers.h
> > +++ b/include/linux/string_helpers.h
> > @@ -71,4 +71,87 @@ 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_NP		0x10
> > +#define ESCAPE_ANY_NP		(ESCAPE_ANY | ESCAPE_NP)
> > +#define ESCAPE_HEX		0x20
> > +
> > +/**
> > + * string_escape_mem - quote characters in the given memory buffer
> 
> It drive me nuts when the kerneldoc is in the .h file.  Who thinks of
> looking there?  I realise that string_unescape() already did that, but
> I'd prefer that we fix string_unescape() rather than imitate it.

No problem, I will do this in separate patch.

> 
> > --- a/lib/string_helpers.c
> > +++ b/lib/string_helpers.c
> 
> This is a lot of code!  Adds nearly a kbyte.  I'm surprised that
> escaping a string is so verbose.
> I wonder if the implementation really needs to be so comprehensive?

I studied several kernel cases and try to cover most of them.

> Would a table-driven approach be more compact?

Do you mean something like ctype set of functions?

I wouldn't think so, we have a lot of variations how we would like to
escape. Currently I have such cases covered in terms of flags I
introduced (not all of them in this series):
 - ESCAPE_OCTAL (with dictionary)
 - ESCAPE_ANY_NP
 - ESCAPE_HEX | ESCAPE_NP
 - ESCAPE_NULL
... not yet in form of patch
 - ESCAPE_SPACE | ESCAPE_SPECIAL (with dictionary)

I can't currently realize how table could cover all of that.

> >  static int __init test_string_helpers_init(void)
> >  {
> >  	unsigned int i;
> > @@ -112,6 +315,16 @@ static int __init test_string_helpers_init(void)
> >  	test_string_unescape("unescape inplace",
> >  			     get_random_int() % (UNESCAPE_ANY + 1), true);
> >  
> > +	/* Without dictionary */
> > +	for (i = 0; i < (ESCAPE_ANY_NP | ESCAPE_HEX) + 1; i++)
> > +		test_string_escape("escape 0", escape0, i, TEST_STRING_2_DICT_0);
> > +
> > +	/* With dictionary */
> > +	for (i = 0; i < (ESCAPE_ANY_NP | ESCAPE_HEX) + 1; i++)
> > +		test_string_escape("escape 1", escape1, i, TEST_STRING_2_DICT_1);
> > +
> > +	test_string_escape_nomem();
> > +
> >  	return -EINVAL;
> >  }
> 
> I wonder why this returns -EINVAL.

The idea of course to not let module be loaded. I saw this return code
in test_kstrtox.c. Would you suggest better approach?


-- 
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/6] lib80211: re-use string_escape_mem_any_np()
  2014-07-02 16:30       ` Joe Perches
@ 2014-07-03 10:49         ` Andy Shevchenko
  0 siblings, 0 replies; 17+ messages in thread
From: Andy Shevchenko @ 2014-07-03 10:49 UTC (permalink / raw)
  To: Joe Perches
  Cc: John W. Linville, Johannes Berg, devel, linux-wireless,
	linux-kernel, Greg Kroah-Hartman, Andrew Morton

On Wed, 2014-07-02 at 09:30 -0700, Joe Perches wrote:
> On Wed, 2014-07-02 at 17:06 +0300, Andy Shevchenko wrote:
> > On Wed, 2014-07-02 at 06:43 -0700, Joe Perches wrote:
> > > On Wed, 2014-07-02 at 16:20 +0300, Andy Shevchenko wrote:
> > > > In kernel we have function to escape a given string. Let's use it instead of
> > > > custom approach.
> > > > 
> > > > This fixes a bug. The current implementation wrongly prints octal numbers: only
> > > > two first digits are used in case when 3 are required and the rest of the
> > > > string ends up cut off.
> > 
> > []
> > 
> > > This code looks like it was adapted from the old print_mac
> > > ethernet code that was eventually replaced by a vsprintf
> > > pointer extension %pM
> > > 
> > > So a better way to do this might be to add and use yet
> > > another vsprintf %p<foo> extension for ssids.
> > 
> > Might be, but it
> >  - doesn't reduce necessity of string_escape_mem (not only ssid are
> > escaped in kernel)
> >  - prevents user to choose a rule what exactly their would like to
> > escape (look at the other patches against ssid escaping)
> > 
> 
> %pE<FLAGS> would allow the same arbitrary combinations
> of your flags.

Makes sense. 
Can do this as a separate patch.
Though I would have a sane default for escaping SSIDs. I hope that one
from print_ssid() could be the one.

-- 
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy


^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2014-07-03 10:49 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-02 13:20 [PATCH 0/6] lib / string_helpers: introduce string_escape_mem Andy Shevchenko
2014-07-02 13:20 ` [PATCH 1/6] lib / string_helpers: clean up test suite Andy Shevchenko
2014-07-02 21:54   ` Andrew Morton
2014-07-03  9:34     ` Andy Shevchenko
2014-07-02 13:20 ` [PATCH 2/6] lib / string_helpers: introduce string_escape_mem() Andy Shevchenko
2014-07-02 22:01   ` Andrew Morton
2014-07-03  9:53     ` Andy Shevchenko
2014-07-02 13:20 ` [PATCH 3/6] lib80211: re-use string_escape_mem_any_np() Andy Shevchenko
2014-07-02 13:43   ` Joe Perches
2014-07-02 14:06     ` Andy Shevchenko
2014-07-02 16:30       ` Joe Perches
2014-07-03 10:49         ` Andy Shevchenko
2014-07-02 13:20 ` [PATCH 4/6] staging: wlan-ng: re-use string_escape_mem() Andy Shevchenko
2014-07-02 13:20 ` [PATCH 5/6] staging: rtl8192e: " Andy Shevchenko
2014-07-02 13:35   ` Joe Perches
2014-07-02 14:11     ` Andy Shevchenko
2014-07-02 13:20 ` [PATCH 6/6] staging: rtl8192u: " Andy Shevchenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).