On 09/12/2020 14:15, Petr Mladek wrote:
On Mon 2020-11-30 14:57:58, Richard Fitzgerald wrote:
Adds test_sscanf to test various number conversion cases, as
number conversion was previously broken.
This also tests the simple_strtoxxx() functions exported from
vsprintf.c.
It is impressive.
Honestly, I do not feel to be expert on testing and mathematics.
I am not sure how comprehensive the test is. Also I am not
sure what experts would say about the tricks with random
numbers.
Anyway, this is much more than what I have expected. And it checks
great number of variants and corner cases.
I suggest only one small change, see below.
--- /dev/null
+++ b/lib/test_scanf.c
@@ -0,0 +1,747 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Test cases for sscanf facility.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/bitops.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/overflow.h>
+#include <linux/printk.h>
+#include <linux/random.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+
+#include "../tools/testing/selftests/kselftest_module.h"
+
+#define BUF_SIZE 1024
+
+static unsigned total_tests __initdata;
+static unsigned failed_tests __initdata;
+static char *test_buffer __initdata;
+static char *fmt_buffer __initdata;
+static struct rnd_state rnd_state __initdata;
+
+typedef int (*check_fn)(const void *check_data, const char *string,
+ const char *fmt, int n_args, va_list ap);
+
+static void __scanf(4, 6) __init
+_test(check_fn fn, const void *check_data, const char *string, const char *fmt,
+ int n_args, ...)
+{
+ va_list ap, ap_copy;
+ int ret;
+
+ total_tests++;
+
+ va_start(ap, n_args);
+ va_copy(ap_copy, ap);
+ ret = vsscanf(string, fmt, ap_copy);
+ va_end(ap_copy);
+
+ if (ret != n_args) {
+ pr_warn("vsscanf(\"%s\", \"%s\", ...) returned %d expected %d\n",
+ string, fmt, ret, n_args);
+ goto fail;
+ }
+
+ ret = (*fn)(check_data, string, fmt, n_args, ap);
+ if (ret)
+ goto fail;
+
+ va_end(ap);
+
+ return;
+
+fail:
+ failed_tests++;
+ va_end(ap);
+}
+
+#define test_one_number(T, gen_fmt, scan_fmt, val, fn) \
+do { \
+ const T expect_val = (T)(val); \
+ T result = ~expect_val; /* should be overwritten */ \
If I get it correctly, this is supposed to initialize the temporary
variable with a value that is different from the expected value.
It will cause test failure when it is not updated by vsscanf().
It does not work for zero value. A better solution might be to add
That's a ~, not a -
~0 = 0xFFFFFFFF
~-1 = 0
a constant, for example:
T result = expect_val + 3; /* do not match when not overwritten */ \
I did not use "+ 1" intentionally because it might hide some overflow
issues.
+ \
+ snprintf(test_buffer, BUF_SIZE, gen_fmt, expect_val); \
+ _test(fn, &expect_val, test_buffer, "%" scan_fmt, 1, &result); \
+} while (0)
Otherwise, it looks good to me.
Best Regards,
Petr
