Hello,
this is an reiteration of my patch from some time ago that introduced
%dE with the same semantic. Back then this resulted in the support for
%pe which was less contentious.
I still consider %de (now with a small 'e' to match %pe) useful.
One concern back then was that drivers/staging/speakup/speakup_bns.c
uses sprintf with the format string "\x05%dE" to produce binary data
expecting a literal 'E'. This is now addressed: ` can be used to end
parsing a format specifier as explained in the commit log of the first
patch.
Of course the concern to not complicate vsprintf() cannot be addressed
as new code is necessary to support this new ability. I still consider
%de useful enough, even though you could do
pr_info("blablub: %pe\n", ERR_PTR(ret));
with the same effect as
pr_info("blablub: %de\n", ret);
.
The second patch converts some strings in the driver core to use this
new format specifier.
Uwe Kleine-König (2):
printf: add support for printing symbolic error names from numbers
driver core: convert probe error messages to use %de
drivers/base/dd.c | 10 +++++-----
lib/test_printf.c | 8 ++++++++
lib/vsprintf.c | 34 +++++++++++++++++++++++++++++++++-
3 files changed, 46 insertions(+), 6 deletions(-)
--
2.24.0
This is an extension of the ability introduced in commit 57f5677e535b
("printf: add support for printing symbolic error names") that
made %pe consume an error valued pointer and emit a symbolic error name.
Here the same is done for numbers:
printk("%de\n", -EIO);
now emits "-EIO\n".
To keep printk flexible enough to emit an 'e' after a number the
character ` can be used which is just swallowed by *printf and ends
interpreting the format code. So
printk("%d`e\n", -5);
emits "-5e\n". (Note that the implementation of ` isn't complete, it
only works for numbers for now. It might make sense to implement the
same for %s and %p.)
For non-error valued numbers %de falls back to emit the plain number (as
%d would do).
Some runtime tests are added to cover %de.
Signed-off-by: Uwe Kleine-König <[email protected]>
---
lib/test_printf.c | 8 ++++++++
lib/vsprintf.c | 34 +++++++++++++++++++++++++++++++++-
2 files changed, 41 insertions(+), 1 deletion(-)
diff --git a/lib/test_printf.c b/lib/test_printf.c
index 2d9f520d2f27..a18a7742d5fe 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -13,6 +13,7 @@
#include <linux/rtc.h>
#include <linux/slab.h>
#include <linux/string.h>
+#include <linux/stringify.h>
#include <linux/bitmap.h>
#include <linux/dcache.h>
@@ -628,6 +629,8 @@ static void __init
errptr(void)
{
test("-1234", "%pe", ERR_PTR(-1234));
+ test("-4321", "%de", -4321);
+ test("-5e", "%d`e", -5);
/* Check that %pe with a non-ERR_PTR gets treated as ordinary %p. */
BUILD_BUG_ON(IS_ERR(PTR));
@@ -641,6 +644,11 @@ errptr(void)
test("[-EIO ]", "[%-8pe]", ERR_PTR(-EIO));
test("[ -EIO]", "[%8pe]", ERR_PTR(-EIO));
test("-EPROBE_DEFER", "%pe", ERR_PTR(-EPROBE_DEFER));
+
+ test("-ERESTARTSYS", "%de", -ERESTARTSYS);
+#else
+
+ test("-" __stringify(ERESTARTSYS), "%de", -ERESTARTSYS);
#endif
}
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 7c488a1ce318..1bcd1ce2c319 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -2637,7 +2637,39 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
num = va_arg(args, unsigned int);
}
- str = number(str, end, num, spec);
+ if (*fmt != 'e') {
+ str = number(str, end, num, spec);
+ } else {
+ unsigned long num_err = 0;
+
+ fmt++;
+
+ /*
+ * error values are negative numbers near zero.
+ * If num represents such a number, it must be
+ * big (as it is unsigned), otherwise print it
+ * as an ordinary number.
+ */
+ if (num > (unsigned long long)-UINT_MAX)
+ num_err = num;
+
+ if (IS_ENABLED(CONFIG_SYMBOLIC_ERRNAME) &&
+ IS_ERR_VALUE(num_err))
+ str = err_ptr(str, end, ERR_PTR(num), spec);
+ else
+ str = number(str, end, num, spec);
+ }
+
+ if (*fmt == '`')
+ /*
+ * When a format specifier is followed by `,
+ * this ends parsing. This way a string can be
+ * printed that has an int followed by a literal
+ * 'e' (using "%d`e") which otherwise (i.e. by
+ * using "%de") would be interpreted as request
+ * to format the int as error code.
+ */
+ ++fmt;
}
}
--
2.24.0
On Mon, 2020-01-20 at 09:55 +0100, Uwe Kleine-K?nig wrote:
> Hello,
>
> this is an reiteration of my patch from some time ago that introduced
> %dE with the same semantic. Back then this resulted in the support for
> %pe which was less contentious.
>
> I still consider %de (now with a small 'e' to match %pe) useful.
I still think this is not a good idea at all.
There really should be no need to introduce more
odd vsprintf extensions.
%p<foo> should be enough.
On Mon, Jan 20, 2020 at 10:57 AM Uwe Kleine-König
<[email protected]> wrote:
> this is an reiteration of my patch from some time ago that introduced
> %dE with the same semantic. Back then this resulted in the support for
> %pe which was less contentious.
>
> I still consider %de (now with a small 'e' to match %pe) useful.
Please, don't spread the extensions over the standard specifiers. The
%p* extensions are enough for my opinion.
NAK.
--
With Best Regards,
Andy Shevchenko
On 20/01/2020 10.32, Andy Shevchenko wrote:
> On Mon, Jan 20, 2020 at 10:57 AM Uwe Kleine-König
> <[email protected]> wrote:
>
>> this is an reiteration of my patch from some time ago that introduced
>> %dE with the same semantic. Back then this resulted in the support for
>> %pe which was less contentious.
>>
>
>> I still consider %de (now with a small 'e' to match %pe) useful.
>
> Please, don't spread the extensions over the standard specifiers. The
> %p* extensions are enough for my opinion.
> NAK.
>
As I think I already mentioned (and what led me to do the %pe), I'm with
Andy and Joe here, I don't think modifying the behaviour of stuff other
than %p is a good idea - especially not when it's only about saving a
few characters to avoid the ERR_PTR() wrapping.
Rasmus
On Tue 2020-01-21 11:27:34, Rasmus Villemoes wrote:
> On 20/01/2020 10.32, Andy Shevchenko wrote:
> > On Mon, Jan 20, 2020 at 10:57 AM Uwe Kleine-K?nig
> > <[email protected]> wrote:
> >
> >> this is an reiteration of my patch from some time ago that introduced
> >> %dE with the same semantic. Back then this resulted in the support for
> >> %pe which was less contentious.
> >>
> >
> >> I still consider %de (now with a small 'e' to match %pe) useful.
> >
> > Please, don't spread the extensions over the standard specifiers. The
> > %p* extensions are enough for my opinion.
> > NAK.
> >
>
> As I think I already mentioned (and what led me to do the %pe), I'm with
> Andy and Joe here, I don't think modifying the behaviour of stuff other
> than %p is a good idea - especially not when it's only about saving a
> few characters to avoid the ERR_PTR() wrapping.
+1
Best Regards,
Petr