Hi Qu,
On Tue, Jan 2, 2024 at 9:56 PM Qu Wenruo <quwenruo.btrfs@xxxxxxx> wrote:
On 2024/1/2 23:53, Geert Uytterhoeven wrote:
On Tue, Jan 2, 2024 at 5:13 AM Qu Wenruo <wqu@xxxxxxxx> wrote:
The new tests cases for memparse_safe() include:
- The existing test cases for kstrtoull()
Including all the 3 bases (8, 10, 16), and all the ok and failure
cases.
Although there are something we need to verify specific for
memparse_safe():
* @retptr and @value are not modified for failure cases
* return value are correct for failure cases
* @retptr is correct for the good cases
- New test cases
Not only testing the result value, but also the @retptr, including:
* good cases with extra tailing chars, but without valid prefix
The @retptr should point to the first char after a valid string.
3 cases for all the 3 bases.
* good cases with extra tailing chars, with valid prefix
5 cases for all the suffixes.
* bad cases without any number but stray suffix
Should be rejected with -EINVAL
Signed-off-by: Qu Wenruo <wqu@xxxxxxxx>
Thanks for your patch!
--- a/lib/test-kstrtox.c
+++ b/lib/test-kstrtox.c
@@ -268,6 +268,237 @@ static void __init test_kstrtoll_ok(void)
TEST_OK(kstrtoll, long long, "%lld", test_ll_ok);
}
+/*
+ * The special pattern to make sure the result is not modified for error cases.
+ */
+#define ULL_PATTERN (0xefefefef7a7a7a7aULL)
+#if BITS_PER_LONG == 32
+#define POINTER_PATTERN (0xefef7a7a7aUL)
This pattern needs 40 bits to fit, so it doesn't fit in a 32-bit
unsigned long or pointer. Probably you wanted to use 0xef7a7a7aUL
instead?
My bad, one extra byte...
So did that fix the sparse warning? ;-)
+#else
+#define POINTER_PATTERN (ULL_PATTERN)
+#endif
Shouldn't a simple cast to uintptr_t work fine for both 32-bit and
64-bit systems:
#define POINTER_PATTERN ((uintptr_t)ULL_PATTERN)
Or even better, incorporate the cast to a pointer:
#define POINTER_PATTERN ((void *)(uintptr_t)ULL_PATTERN)
The problem is reported by sparse, which warns about that ULL_PATTERN
converted to a pointer would lose its width:
lib/test-kstrtox.c:339:40: sparse: sparse: cast truncates bits from
constant value (efefefef7a7a7a7a becomes 7a7a7a7a)
Ah yes, sparse can be annoying.
I'm still looking for a clean and concise way to shut up [1].
I'm not sure if using uiintptr_t would solve it, thus I go the macro to
switch the value to avoid the static checker's warning.
I tried to check how other locations handles patterned pointer value,
like CONFIG_INIT_STACK_ALL_PATTERN, but they're either relying on the
compiler or just memset().
Any better idea to solve the problem in a better way?
Masking off the extra bits, like lower_32_bits()[2] does?
#define POINTER_PATTERN ((void *)(uintptr_t)((ULL_PATTERN) & UINTPTR_MAX))
[1] https://lore.kernel.org/oe-kbuild-all/202312181649.u6k6hLIm-lkp@xxxxxxxxx/
[2] https://elixir.bootlin.com/linux/latest/source/include/linux/kernel.h#L82
Gr{oetje,eeting}s,
Geert
Attachment:
OpenPGP_0xC23D91F3A125FEA8.asc
Description: OpenPGP public key
Attachment:
OpenPGP_signature.asc
Description: OpenPGP digital signature