[PATCH 0/2] printf: add support for %de

[PATCH 0/2] printf: add support for %de

[Uwe Kleine-König]

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

[PATCH 1/2] printf: add support for printing symbolic error names from numbers

[Uwe Kleine-König]

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 <u.kleine-koenig@pengutronix.de>
---
 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

[PATCH 2/2] driver core: convert probe error messages to use %de

[Uwe Kleine-König]

With this change

	rtc-s35390a: probe of 3-0030 failed with error -EIO

is emitted instead of

	rtc-s35390a: probe of 3-0030 failed with error -5

. The former is more descriptive and so gives a better hint what is
actually broken. The actual value of the error code isn't of any
importance, so there is no relevant information lost.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/base/dd.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index d811e60610d3..e4e308ac187b 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -620,13 +620,13 @@ static int really_probe(struct device *dev, struct device_driver *drv)
 		break;
 	case -ENODEV:
 	case -ENXIO:
-		pr_debug("%s: probe of %s rejects match %d\n",
+		pr_debug("%s: probe of %s rejects match %de\n",
 			 drv->name, dev_name(dev), ret);
 		break;
 	default:
 		/* driver matched but the probe failed */
 		printk(KERN_WARNING
-		       "%s: probe of %s failed with error %d\n",
+		       "%s: probe of %s failed with error %de\n",
 		       drv->name, dev_name(dev), ret);
 	}
 	/*
@@ -652,7 +652,7 @@ static int really_probe_debug(struct device *dev, struct device_driver *drv)
 	ret = really_probe(dev, drv);
 	rettime = ktime_get();
 	delta = ktime_sub(rettime, calltime);
-	printk(KERN_DEBUG "probe of %s returned %d after %lld usecs\n",
+	printk(KERN_DEBUG "probe of %s returned %de after %lld usecs\n",
 	       dev_name(dev), ret, (s64) ktime_to_us(delta));
 	return ret;
 }
@@ -813,7 +813,7 @@ static int __device_attach_driver(struct device_driver *drv, void *_data)
 		dev_dbg(dev, "Device match requests probe deferral\n");
 		driver_deferred_probe_add(dev);
 	} else if (ret < 0) {
-		dev_dbg(dev, "Bus failed to match device: %d", ret);
+		dev_dbg(dev, "Bus failed to match device: %de", ret);
 		return ret;
 	} /* ret > 0 means positive match */
 
@@ -1046,7 +1046,7 @@ static int __driver_attach(struct device *dev, void *data)
 		dev_dbg(dev, "Device match requests probe deferral\n");
 		driver_deferred_probe_add(dev);
 	} else if (ret < 0) {
-		dev_dbg(dev, "Bus failed to match device: %d", ret);
+		dev_dbg(dev, "Bus failed to match device: %de", ret);
 		return ret;
 	} /* ret > 0 means positive match */
 
-- 
2.24.0

Re: [PATCH 0/2] printf: add support for %de

[Joe Perches]

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.

Re: [PATCH 0/2] printf: add support for %de

[Andy Shevchenko]

On Mon, Jan 20, 2020 at 10:57 AM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> 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

Re: [PATCH 0/2] printf: add support for %de

[Rasmus Villemoes]

On 20/01/2020 10.32, Andy Shevchenko wrote:
> On Mon, Jan 20, 2020 at 10:57 AM Uwe Kleine-König
> <u.kleine-koenig@pengutronix.de> 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

Re: [PATCH 0/2] printf: add support for %de

[Petr Mladek]

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
> > <u.kleine-koenig@pengutronix.de> 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