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 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