Re: [PATCH] vsprintf: sanely handle NULL passed to %pe
From: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Date: 2020-02-19 13:56:37
Also in:
lkml
On 19/02/2020 14.48, Petr Mladek wrote:
On Wed 2020-02-19 12:53:22, Rasmus Villemoes wrote:quoted
--- a/lib/vsprintf.c +++ b/lib/vsprintf.c@@ -619,7 +619,7 @@ static char *err_ptr(char *buf, char *end, void *ptr, struct printf_spec spec) { int err = PTR_ERR(ptr); - const char *sym = errname(err); + const char *sym = err ? errname(err) : "NULL";I like this more than adding "NULL" errname.
OK.
quoted
if (sym) return string_nocheck(buf, end, sym, spec); instead of the change(s) in errname.c? And then the test case for '"%pe", NULL' should also be moved outside CONFIG_SYMBOLIC_ERRNAME.The test should go into null_pointer() instead of errptr().
Eh, no, the behaviour of %pe is tested by errptr(). I'll keep it that way. But I should add a #else section that tests how %pe behaves without CONFIG_SYMBOLIC_ERRNAME - though that's orthogonal to this patch.
Could you send updated patch, please? ;-)
I'll wait a day or two for more comments. It doesn't seem very urgent.
quoted hunk ↗ jump to hunk
quoted
BTW., your original patch for %p lacks corresponding update of test_vsprintf.c. Please add appropriate test cases.Good point. The existing test_hashed() is rather weak and it did not catch this change. It would be nice to make test_hash() more powerful. Anyway, the minimal udpate would be:diff --git a/lib/test_printf.c b/lib/test_printf.c index 2d9f520d2f27..1726a678bccd 100644 --- a/lib/test_printf.c +++ b/lib/test_printf.c@@ -333,7 +333,7 @@ test_hashed(const char *fmt, const void *p) static void __init null_pointer(void) { - test_hashed("%p", NULL); + test(ZEROS "00000000", "%p", NULL);
No, it most certainly also needs to check a few "%p", ERR_PTR(-4) cases (where one of course has to use explicit integers and not E* constants). Rasmus