On Fri 2019-10-11 15:36:17, Rasmus Villemoes wrote: > It has been suggested several times to extend vsnprintf() to be able > to convert the numeric value of ENOSPC to print "ENOSPC". This > implements that as a %p extension: With %pe, one can do
Reviewed-by: Petr Mladek <[email protected]> I like the patch. There are only two rather cosmetic things. > diff --git a/lib/errname.c b/lib/errname.c > new file mode 100644 > index 000000000000..30d3bab99477 > --- /dev/null > +++ b/lib/errname.c > +const char *errname(int err) > +{ > + bool pos = err > 0; > + const char *name = __errname(err > 0 ? err : -err); > + > + return name ? name + pos : NULL; This made me to check C standard. It seems that "true" really has to be "1". But I think that I am not the only one who is not sure. I would prefer to make it less tricky and use, for example: const char *name = __errname(err > 0 ? err : -err); if (!name) return NULL; return err > 0 ? name + 1 : name; > +} > diff --git a/lib/test_printf.c b/lib/test_printf.c > index 5d94cbff2120..4fa0ccf58420 100644 > --- a/lib/test_printf.c > +++ b/lib/test_printf.c > @@ -593,6 +593,29 @@ flags(void) > kfree(cmp_buffer); > } > > +static void __init > +errptr(void) > +{ > + char buf[PLAIN_BUF_SIZE]; > + > + test("-1234", "%pe", ERR_PTR(-1234)); > + > + /* Check that %pe with a non-ERR_PTR gets treated as ordinary %p. */ > + BUILD_BUG_ON(IS_ERR(PTR)); > + snprintf(buf, sizeof(buf), "(%p)", PTR); > + test(buf, "(%pe)", PTR); There is a small race. "(____ptrval____)" is used for %p before random numbers are initialized. The switch is done via workqueue work, see enable_ptr_key_workfn(). It means that it can be done in parallel. I doubt that anyone would ever hit the race. But it could be very confusing and hard to debug. I would replace it with: test_hashed("%pe", PTR); If would like to have the two things fixed. I am not sure if you want to send one more revision. Or I could also change it by follow up patch when pushing. What is your preference, please? Best Regards, Petr
