Skip to contentSkip to conversation

hexdump: amend the hex_dump_to_buffer()

PATCH In review Andy Shevchenko 2 participants 6 messages Dec 29, 2014 – Jan 12, 2015

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover hexdump: amend the hex_dump_to_buffer()

This series is targeted to amend hex_dump_to_buffer() function. It consists test suite, fix for output format and features the function by adding return code.

With those changes the function would be used for seq_file API.

Andy Shevchenko (4):
hexdump: introduce test suite
hexdump: fix ascii column for the tail of a dump
hexdump: do few calculations ahead
hexdump: makes it return amount of bytes placed in buffer

 include/linux/printk.h |   6 +-
 lib/Kconfig.debug      |   3 +
 lib/Makefile           |   4 +-
 lib/hexdump.c          | 105 ++++++++++++++++++-----------
 lib/test-hexdump.c     | 180 +++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 253 insertions(+), 45 deletions(-)
 create mode 100644 lib/test-hexdump.c
Signature · 1 line
2.1.3
#1 lore raw

PATCH 1/4 hexdump: introduce test suite

Test different scenarios of function calls located in lib/hexdump.c.

Currently hex_dump_to_buffer() is only tested and test data is provided for little endian CPUs.

---
 lib/Kconfig.debug  |   3 ++
 lib/Makefile       |   4 +-
 lib/test-hexdump.c | 135 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 141 insertions(+), 1 deletion(-)
 create mode 100644 lib/test-hexdump.c
lib/Kconfig.debug +3 -0
index a4a6d9a..5fe0a71 100644
@@ -1580,6 +1580,9 @@ config ASYNC_RAID6_TEST
 
 	  If unsure, say N.
 
+config TEST_HEXDUMP
+	tristate "Test functions located in the hexdump module at runtime"
+
 config TEST_STRING_HELPERS
 	tristate "Test functions located in the string_helpers module at runtime"
 
lib/Makefile +3 -1
index f42838a..3dca86f 100644
@@ -23,12 +23,14 @@ lib-y	+= kobject.o klist.o
 obj-y	+= lockref.o
 
 obj-y += bcd.o div64.o sort.o parser.o halfmd4.o debug_locks.o random32.o \
-	 bust_spinlocks.o hexdump.o kasprintf.o bitmap.o scatterlist.o \
+	 bust_spinlocks.o kasprintf.o bitmap.o scatterlist.o \
 	 gcd.o lcm.o list_sort.o uuid.o flex_array.o iovec.o clz_ctz.o \
 	 bsearch.o find_last_bit.o find_next_bit.o llist.o memweight.o kfifo.o \
 	 percpu-refcount.o percpu_ida.o rhashtable.o reciprocal_div.o
 obj-y += string_helpers.o
 obj-$(CONFIG_TEST_STRING_HELPERS) += test-string_helpers.o
+obj-y += hexdump.o
+obj-$(CONFIG_TEST_HEXDUMP) += test-hexdump.o
 obj-y += kstrtox.o
 obj-$(CONFIG_TEST_KSTRTOX) += test-kstrtox.o
 obj-$(CONFIG_TEST_LKM) += test_module.o
lib/test-hexdump.c new file +135 -0

135 lines not on this page · raw message

Signature · 1 line
2.1.3
#2 lore raw

Re: [PATCH v1 1/4] hexdump: introduce test suite

Quoted 18 lines · Andy Shevchenko

Test different scenarios of function calls located in lib/hexdump.c.

Currently hex_dump_to_buffer() is only tested and test data is provided for little endian CPUs.

---
 lib/Kconfig.debug  |   3 ++
 lib/Makefile       |   4 +-
 lib/test-hexdump.c | 135 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 141 insertions(+), 1 deletion(-)
 create mode 100644 lib/test-hexdump.c
lib/Kconfig.debug +0 -0
index a4a6d9a..5fe0a71 100644

lib/Kconfig.debug config ASYNC_RAID6_TEST · line 1585

Quoted 6 lines
lib/Kconfig.debug +3 -0
@@ -1580,6 +1580,9 @@ config ASYNC_RAID6_TEST
 
          If unsure, say N.
 
+config TEST_HEXDUMP
+       tristate "Test functions located in the hexdump module at runtime"
+

I just happened across this as it appeared in linux-next and came up in "make oldconfig". I realize that there are other options like the one immediately below in the context that have no help text, but I don't think that makes it a good reason to add more w/o any
help text. Just filling in the blanks in something as simple as:

The hexdump module provides library functions like ....
that are primarily used by the ... Enabling this option will
build a simple test run at boot/module insertion that checks...

If unsure, say N.

Thanks,
Paul.
--

Quoted 172 lines
lib/Kconfig.debug +0 -0
 config TEST_STRING_HELPERS
        tristate "Test functions located in the string_helpers module at runtime"
 
lib/Makefile +3 -1
index f42838a..3dca86f 100644
@@ -23,12 +23,14 @@ lib-y       += kobject.o klist.o
 obj-y  += lockref.o
 
 obj-y += bcd.o div64.o sort.o parser.o halfmd4.o debug_locks.o random32.o \
-        bust_spinlocks.o hexdump.o kasprintf.o bitmap.o scatterlist.o \
+        bust_spinlocks.o kasprintf.o bitmap.o scatterlist.o \
         gcd.o lcm.o list_sort.o uuid.o flex_array.o iovec.o clz_ctz.o \
         bsearch.o find_last_bit.o find_next_bit.o llist.o memweight.o kfifo.o \
         percpu-refcount.o percpu_ida.o rhashtable.o reciprocal_div.o
 obj-y += string_helpers.o
 obj-$(CONFIG_TEST_STRING_HELPERS) += test-string_helpers.o
+obj-y += hexdump.o
+obj-$(CONFIG_TEST_HEXDUMP) += test-hexdump.o
 obj-y += kstrtox.o
 obj-$(CONFIG_TEST_KSTRTOX) += test-kstrtox.o
 obj-$(CONFIG_TEST_LKM) += test_module.o
lib/test-hexdump.c new file +135 -1
new file mode 100644
index 0000000..9d3bd1e
@@ -0,0 +1,135 @@
+/*
+ * Test cases for lib/hexdump.c module.
+ */
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/random.h>
+#include <linux/string.h>
+
+static const unsigned char data_b[] = {
+       '\xbe', '\x32', '\xdb', '\x7b', '\x0a', '\x18', '\x93', '\xb2', /* 00 - 07 */
+       '\x70', '\xba', '\xc4', '\x24', '\x7d', '\x83', '\x34', '\x9b', /* 08 - 0f */
+       '\xa6', '\x9c', '\x31', '\xad', '\x9c', '\x0f', '\xac', '\xe9', /* 10 - 17 */
+       '\x4c', '\xd1', '\x19', '\x99', '\x43', '\xb1', '\xaf', '\x0c', /* 18 - 1f */
+};
+
+static const unsigned char data_a[] = ".2.{....p..$}.4...1.....L...C...";
+
+static const char *test_data_1_le[] __initconst = {
+       "be", "32", "db", "7b", "0a", "18", "93", "b2",
+       "70", "ba", "c4", "24", "7d", "83", "34", "9b",
+       "a6", "9c", "31", "ad", "9c", "0f", "ac", "e9",
+       "4c", "d1", "19", "99", "43", "b1", "af", "0c",
+};
+
+static const char *test_data_2_le[] __initconst = {
+       "32be", "7bdb", "180a", "b293",
+       "ba70", "24c4", "837d", "9b34",
+       "9ca6", "ad31", "0f9c", "e9ac",
+       "d14c", "9919", "b143", "0caf",
+};
+
+static const char *test_data_4_le[] __initconst = {
+       "7bdb32be", "b293180a", "24c4ba70", "9b34837d",
+       "ad319ca6", "e9ac0f9c", "9919d14c", "0cafb143",
+};
+
+static const char *test_data_8_le[] __initconst = {
+       "b293180a7bdb32be", "9b34837d24c4ba70",
+       "e9ac0f9cad319ca6", "0cafb1439919d14c",
+};
+
+static void __init test_hexdump(size_t len, int rowsize, int groupsize,
+                               bool ascii)
+{
+       char test[32 * 3 + 2 + 32 + 1];
+       char real[32 * 3 + 2 + 32 + 1];
+       char *p;
+       const char **result;
+       size_t l = len;
+       int gs = groupsize, rs = rowsize;
+       unsigned int i;
+
+       hex_dump_to_buffer(data_b, l, rs, gs, real, sizeof(real), ascii);
+
+       if (rs != 16 && rs != 32)
+               rs = 16;
+
+       if (l > rs)
+               l = rs;
+
+       if (!is_power_of_2(gs) || gs > 8 || (len % gs != 0))
+               gs = 1;
+
+       if (gs == 8)
+               result = test_data_8_le;
+       else if (gs == 4)
+               result = test_data_4_le;
+       else if (gs == 2)
+               result = test_data_2_le;
+       else
+               result = test_data_1_le;
+
+       memset(test, ' ', sizeof(test));
+
+       /* hex dump */
+       p = test;
+       for (i = 0; i < l / gs; i++) {
+               const char *q = *result++;
+               size_t amount = strlen(q);
+
+               strncpy(p, q, amount);
+               p += amount + 1;
+       }
+       if (i)
+               p--;
+
+       /* ASCII part */
+       if (ascii) {
+               p = test + rs * 2 + rs / gs + 1;
+               strncpy(p, data_a, l);
+               p += l;
+       }
+
+       *p = '\0';
+
+       if (strcmp(test, real)) {
+               pr_err("Len: %zu row: %d group: %d\n", len, rowsize, groupsize);
+               pr_err("Result: '%s'\n", real);
+               pr_err("Expect: '%s'\n", test);
+       }
+}
+
+static void __init test_hexdump_set(int rowsize, bool ascii)
+{
+       size_t d = min_t(size_t, sizeof(data_b), rowsize);
+       size_t len = get_random_int() % d + 1;
+
+       test_hexdump(len, rowsize, 4, ascii);
+       test_hexdump(len, rowsize, 2, ascii);
+       test_hexdump(len, rowsize, 8, ascii);
+       test_hexdump(len, rowsize, 1, ascii);
+}
+
+static int __init test_hexdump_init(void)
+{
+       unsigned int i;
+       int rowsize;
+
+       pr_info("Running tests...\n");
+
+       rowsize = (get_random_int() % 2 + 1) * 16;
+       for (i = 0; i < 16; i++)
+               test_hexdump_set(rowsize, false);
+
+       rowsize = (get_random_int() % 2 + 1) * 16;
+       for (i = 0; i < 16; i++)
+               test_hexdump_set(rowsize, true);
+
+       return -EINVAL;
+}
+module_init(test_hexdump_init);
+MODULE_LICENSE("Dual BSD/GPL");
--

2.1.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

#3 lore raw

PATCH 2/4 hexdump: fix ascii column for the tail of a dump

In reply to Andy Shevchenko

In current implementation we have floating ascii column in the tail of the dump.

For example, for row size equal to 16 the ascii column as in following table

group size \ length 8 12 16

	1		50	50	50
	2		22	32	42
	4		20	29	38
	8		19	-	36

This patch makes it the same independently of amount of bytes dumped.

The change is safe since all current users, which use ASCII part of the dump, rely on the group size equal to 1. The patch doesn't change behaviour for such group size (see the table above).

---
 lib/hexdump.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
lib/hexdump.c +3 -3
index 270773b..a61cb6b 100644
@@ -126,7 +126,7 @@ void hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
 			lx += scnprintf(linebuf + lx, linebuflen - lx,
 					"%s%16.16llx", j ? " " : "",
 					(unsigned long long)*(ptr8 + j));
-		ascii_column = 17 * ngroups + 2;
+		ascii_column = rowsize * 2 + rowsize / 8 + 2;
 		break;
 	}
 
@@ -137,7 +137,7 @@ void hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
 		for (j = 0; j < ngroups; j++)
 			lx += scnprintf(linebuf + lx, linebuflen - lx,
 					"%s%8.8x", j ? " " : "", *(ptr4 + j));
-		ascii_column = 9 * ngroups + 2;
+		ascii_column = rowsize * 2 + rowsize / 4 + 2;
 		break;
 	}
 
@@ -148,7 +148,7 @@ void hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
 		for (j = 0; j < ngroups; j++)
 			lx += scnprintf(linebuf + lx, linebuflen - lx,
 					"%s%4.4x", j ? " " : "", *(ptr2 + j));
-		ascii_column = 5 * ngroups + 2;
+		ascii_column = rowsize * 2 + rowsize / 2 + 2;
 		break;
 	}
 
Signature · 1 line
2.1.3
#4 lore raw

PATCH 3/4 hexdump: do few calculations ahead

In reply to Andy Shevchenko

Instead of doing calculations in each case of different groupsize let's do them beforehand. While here, change the switch by if-else-if construction.

---
 lib/hexdump.c | 34 ++++++++++------------------------
 1 file changed, 10 insertions(+), 24 deletions(-)
lib/hexdump.c +10 -24
index a61cb6b..4af53f7 100644
@@ -103,6 +103,7 @@ void hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
 			bool ascii)
 {
 	const u8 *ptr = buf;
+	int ngroups;
 	u8 ch;
 	int j, lx = 0;
 	int ascii_column;
@@ -114,45 +115,33 @@ void hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
 		goto nil;
 	if (len > rowsize)		/* limit to one line at a time */
 		len = rowsize;
+	if (!is_power_of_2(groupsize) || groupsize > 8)
+		groupsize = 1;
 	if ((len % groupsize) != 0)	/* no mixed size output */
 		groupsize = 1;
 
-	switch (groupsize) {
-	case 8: {
+	ngroups = len / groupsize;
+	ascii_column = rowsize * 2 + rowsize / groupsize + 1;
+	if (groupsize == 8) {
 		const u64 *ptr8 = buf;
-		int ngroups = len / groupsize;
 
 		for (j = 0; j < ngroups; j++)
 			lx += scnprintf(linebuf + lx, linebuflen - lx,
 					"%s%16.16llx", j ? " " : "",
 					(unsigned long long)*(ptr8 + j));
-		ascii_column = rowsize * 2 + rowsize / 8 + 2;
-		break;
-	}
-
-	case 4: {
+	} else if (groupsize == 4) {
 		const u32 *ptr4 = buf;
-		int ngroups = len / groupsize;
 
 		for (j = 0; j < ngroups; j++)
 			lx += scnprintf(linebuf + lx, linebuflen - lx,
 					"%s%8.8x", j ? " " : "", *(ptr4 + j));
-		ascii_column = rowsize * 2 + rowsize / 4 + 2;
-		break;
-	}
-
-	case 2: {
+	} else if (groupsize == 2) {
 		const u16 *ptr2 = buf;
-		int ngroups = len / groupsize;
 
 		for (j = 0; j < ngroups; j++)
 			lx += scnprintf(linebuf + lx, linebuflen - lx,
 					"%s%4.4x", j ? " " : "", *(ptr2 + j));
-		ascii_column = rowsize * 2 + rowsize / 2 + 2;
-		break;
-	}
-
-	default:
+	} else {
 		for (j = 0; (j < len) && (lx + 3) <= linebuflen; j++) {
 			ch = ptr[j];
 			linebuf[lx++] = hex_asc_hi(ch);
@@ -161,14 +150,11 @@ void hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
 		}
 		if (j)
 			lx--;
-
-		ascii_column = 3 * rowsize + 2;
-		break;
 	}
 	if (!ascii)
 		goto nil;
 
-	while (lx < (linebuflen - 1) && lx < (ascii_column - 1))
+	while (lx < (linebuflen - 1) && lx < ascii_column)
 		linebuf[lx++] = ' ';
 	for (j = 0; (j < len) && (lx + 2) < linebuflen; j++) {
 		ch = ptr[j];
Signature · 1 line
2.1.3
#5 lore raw

PATCH 4/4 hexdump: makes it return amount of bytes placed in buffer

In reply to Andy Shevchenko

This patch makes hexdump to return amount of bytes placed in the buffer excluding trailing NUL. In case of overflow it returns desired amount of bytes to place entire dump. Thus, it mimics snprintf().

This will be useful for users that would like to repeat with bigger buffer.

---
 include/linux/printk.h |  6 ++---
 lib/hexdump.c          | 73 +++++++++++++++++++++++++++++++++++++-------------
 lib/test-hexdump.c     | 45 +++++++++++++++++++++++++++++++
 3 files changed, 103 insertions(+), 21 deletions(-)
include/linux/printk.h +3 -3
index c8f1703..aeb9d7f 100644
@@ -408,9 +408,9 @@ enum {
 	DUMP_PREFIX_ADDRESS,
 	DUMP_PREFIX_OFFSET
 };
-extern void hex_dump_to_buffer(const void *buf, size_t len,
-			       int rowsize, int groupsize,
-			       char *linebuf, size_t linebuflen, bool ascii);
+extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
+			      int groupsize, char *linebuf, size_t linebuflen,
+			      bool ascii);
 #ifdef CONFIG_PRINTK
 extern void print_hex_dump(const char *level, const char *prefix_str,
 			   int prefix_type, int rowsize, int groupsize,
lib/hexdump.c +55 -18

118 lines not on this page · raw message

lib/test-hexdump.c +45 -0

57 lines not on this page · raw message

Signature · 1 line
2.1.3