It has been suggested several times to extend vsnprintf() to be able
to convert the numeric value of ENOSPC to print "ENOSPC". This is yet
another attempt. Rather than adding another %p extension, simply teach
plain %p to convert ERR_PTRs. While the primary use case is

  if (IS_ERR(foo)) {
    pr_err("Sorry, can't do that: %p\n", foo);
    return PTR_ERR(foo);
  }

it is also more helpful to get a symbolic error code (or, worst case,
a decimal number) in case an ERR_PTR is accidentally passed to some
%p<something>, rather than the (efault) that check_pointer() would
result in.

With my embedded hat on, I've made it possible to remove this.

I've tested that the #ifdeffery in errcode.c is sufficient to make
this compile on arm, arm64, mips, powerpc, s390, x86 - I'm sure the
0day bot will tell me which ones I've missed.

The symbols to include have been found by massaging the output of

  find arch include -iname 'errno*.h' | xargs grep -E 'define\s*E'

In the cases where some common aliasing exists
(e.g. EAGAIN=EWOULDBLOCK on all platforms, EDEADLOCK=EDEADLK on most),
I've moved the more popular one (in terms of 'git grep -w Efoo | wc)
to the bottom so that one takes precedence.

Acked-by: Uwe Kleine-König <u...@kleine-koenig.org>
Signed-off-by: Rasmus Villemoes <li...@rasmusvillemoes.dk>
---

Andrew: please consider picking this up, even if we're already in the
merge window. Quite a few people have said they'd like to see
something like this, it's a debug improvement in its own right (the
"ERR_PTR accidentally passed to printf" case), the printf tests pass,
and it's much easier to start adding (and testing) users around the
tree once this is in master.

v3:
- only accept positive errno values in errcode()
- change type of err variable in pointer() from long to int
v2:
- add #include <linux/stddef.h> to errcode.h (0day)
- keep 'x' handling in switch() (Andy)
- add paragraph to Documentation/core-api/printk-formats.rst
- add ack from Uwe

 Documentation/core-api/printk-formats.rst |   8 +
 include/linux/errcode.h                   |  16 ++
 lib/Kconfig.debug                         |   8 +
 lib/Makefile                              |   1 +
 lib/errcode.c                             | 212 ++++++++++++++++++++++
 lib/test_printf.c                         |  14 ++
 lib/vsprintf.c                            |  26 +++
 7 files changed, 285 insertions(+)
 create mode 100644 include/linux/errcode.h
 create mode 100644 lib/errcode.c

diff --git a/Documentation/core-api/printk-formats.rst 
b/Documentation/core-api/printk-formats.rst
index c6224d039bcb..7d3bf3cb207b 100644
--- a/Documentation/core-api/printk-formats.rst
+++ b/Documentation/core-api/printk-formats.rst
@@ -66,6 +66,14 @@ might be printed instead of the unreachable information::
        (efault) data on invalid address
        (einval) invalid data on a valid address
 
+Error pointers, i.e. pointers for which IS_ERR() is true, are handled
+as follows: If CONFIG_SYMBOLIC_ERRCODE is set, an appropriate symbolic
+constant is printed. For example, '"%p", PTR_ERR(-ENOSPC)' results in
+"ENOSPC", while '"%p", PTR_ERR(-EWOULDBLOCK)' results in "EAGAIN"
+(since EAGAIN == EWOULDBLOCK, and the former is the most common). If
+CONFIG_SYMBOLIC_ERRCODE is not set, ERR_PTRs are printed as their
+decimal representation ("-28" and "-11" for the two examples).
+
 Plain Pointers
 --------------
 
diff --git a/include/linux/errcode.h b/include/linux/errcode.h
new file mode 100644
index 000000000000..c6a4c1b04f9c
--- /dev/null
+++ b/include/linux/errcode.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_ERRCODE_H
+#define _LINUX_ERRCODE_H
+
+#include <linux/stddef.h>
+
+#ifdef CONFIG_SYMBOLIC_ERRCODE
+const char *errcode(int err);
+#else
+static inline const char *errcode(int err)
+{
+       return NULL;
+}
+#endif
+
+#endif /* _LINUX_ERRCODE_H */
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 5960e2980a8a..dc1b20872774 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -164,6 +164,14 @@ config DYNAMIC_DEBUG
          See Documentation/admin-guide/dynamic-debug-howto.rst for additional
          information.
 
+config SYMBOLIC_ERRCODE
+       bool "Support symbolic error codes in printf"
+       help
+         If you say Y here, the kernel's printf implementation will
+         be able to print symbolic errors such as ENOSPC instead of
+         the number 28. It makes the kernel image slightly larger
+         (about 3KB), but can make the kernel logs easier to read.
+
 endmenu # "printk and dmesg options"
 
 menu "Compile-time checks and compiler options"
diff --git a/lib/Makefile b/lib/Makefile
index c5892807e06f..9f14edc7ef63 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -181,6 +181,7 @@ lib-$(CONFIG_GENERIC_BUG) += bug.o
 obj-$(CONFIG_HAVE_ARCH_TRACEHOOK) += syscall.o
 
 obj-$(CONFIG_DYNAMIC_DEBUG) += dynamic_debug.o
+obj-$(CONFIG_SYMBOLIC_ERRCODE) += errcode.o
 
 obj-$(CONFIG_NLATTR) += nlattr.o
 
diff --git a/lib/errcode.c b/lib/errcode.c
new file mode 100644
index 000000000000..3e519b13245e
--- /dev/null
+++ b/lib/errcode.c
@@ -0,0 +1,212 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/build_bug.h>
+#include <linux/errno.h>
+#include <linux/errcode.h>
+#include <linux/kernel.h>
+
+/*
+ * Ensure these tables to not accidentally become gigantic if some
+ * huge errno makes it in. On most architectures, the first table will
+ * only have about 140 entries, but mips and parisc have more sparsely
+ * allocated errnos (with EHWPOISON = 257 on parisc, and EDQUOT = 1133
+ * on mips), so this wastes a bit of space on those - though we
+ * special case the EDQUOT case.
+ */
+#define E(err) [err + BUILD_BUG_ON_ZERO(err <= 0 || err > 300)] = #err
+static const char *codes_0[] = {
+       E(E2BIG),
+       E(EACCES),
+       E(EADDRINUSE),
+       E(EADDRNOTAVAIL),
+       E(EADV),
+       E(EAFNOSUPPORT),
+       E(EALREADY),
+       E(EBADE),
+       E(EBADF),
+       E(EBADFD),
+       E(EBADMSG),
+       E(EBADR),
+       E(EBADRQC),
+       E(EBADSLT),
+       E(EBFONT),
+       E(EBUSY),
+#ifdef ECANCELLED
+       E(ECANCELLED),
+#endif
+       E(ECHILD),
+       E(ECHRNG),
+       E(ECOMM),
+       E(ECONNABORTED),
+       E(ECONNRESET),
+       E(EDEADLOCK),
+       E(EDESTADDRREQ),
+       E(EDOM),
+       E(EDOTDOT),
+#ifndef CONFIG_MIPS
+       E(EDQUOT),
+#endif
+       E(EEXIST),
+       E(EFAULT),
+       E(EFBIG),
+       E(EHOSTDOWN),
+       E(EHOSTUNREACH),
+       E(EHWPOISON),
+       E(EIDRM),
+       E(EILSEQ),
+#ifdef EINIT
+       E(EINIT),
+#endif
+       E(EINPROGRESS),
+       E(EINTR),
+       E(EINVAL),
+       E(EIO),
+       E(EISCONN),
+       E(EISDIR),
+       E(EISNAM),
+       E(EKEYEXPIRED),
+       E(EKEYREJECTED),
+       E(EKEYREVOKED),
+       E(EL2HLT),
+       E(EL2NSYNC),
+       E(EL3HLT),
+       E(EL3RST),
+       E(ELIBACC),
+       E(ELIBBAD),
+       E(ELIBEXEC),
+       E(ELIBMAX),
+       E(ELIBSCN),
+       E(ELNRNG),
+       E(ELOOP),
+       E(EMEDIUMTYPE),
+       E(EMFILE),
+       E(EMLINK),
+       E(EMSGSIZE),
+       E(EMULTIHOP),
+       E(ENAMETOOLONG),
+       E(ENAVAIL),
+       E(ENETDOWN),
+       E(ENETRESET),
+       E(ENETUNREACH),
+       E(ENFILE),
+       E(ENOANO),
+       E(ENOBUFS),
+       E(ENOCSI),
+       E(ENODATA),
+       E(ENODEV),
+       E(ENOENT),
+       E(ENOEXEC),
+       E(ENOKEY),
+       E(ENOLCK),
+       E(ENOLINK),
+       E(ENOMEDIUM),
+       E(ENOMEM),
+       E(ENOMSG),
+       E(ENONET),
+       E(ENOPKG),
+       E(ENOPROTOOPT),
+       E(ENOSPC),
+       E(ENOSR),
+       E(ENOSTR),
+#ifdef ENOSYM
+       E(ENOSYM),
+#endif
+       E(ENOSYS),
+       E(ENOTBLK),
+       E(ENOTCONN),
+       E(ENOTDIR),
+       E(ENOTEMPTY),
+       E(ENOTNAM),
+       E(ENOTRECOVERABLE),
+       E(ENOTSOCK),
+       E(ENOTTY),
+       E(ENOTUNIQ),
+       E(ENXIO),
+       E(EOPNOTSUPP),
+       E(EOVERFLOW),
+       E(EOWNERDEAD),
+       E(EPERM),
+       E(EPFNOSUPPORT),
+       E(EPIPE),
+#ifdef EPROCLIM
+       E(EPROCLIM),
+#endif
+       E(EPROTO),
+       E(EPROTONOSUPPORT),
+       E(EPROTOTYPE),
+       E(ERANGE),
+       E(EREMCHG),
+#ifdef EREMDEV
+       E(EREMDEV),
+#endif
+       E(EREMOTE),
+       E(EREMOTEIO),
+#ifdef EREMOTERELEASE
+       E(EREMOTERELEASE),
+#endif
+       E(ERESTART),
+       E(ERFKILL),
+       E(EROFS),
+#ifdef ERREMOTE
+       E(ERREMOTE),
+#endif
+       E(ESHUTDOWN),
+       E(ESOCKTNOSUPPORT),
+       E(ESPIPE),
+       E(ESRCH),
+       E(ESRMNT),
+       E(ESTALE),
+       E(ESTRPIPE),
+       E(ETIME),
+       E(ETIMEDOUT),
+       E(ETOOMANYREFS),
+       E(ETXTBSY),
+       E(EUCLEAN),
+       E(EUNATCH),
+       E(EUSERS),
+       E(EXDEV),
+       E(EXFULL),
+
+       E(ECANCELED),
+       E(EAGAIN), /* EWOULDBLOCK */
+       E(ECONNREFUSED), /* EREFUSED */
+       E(EDEADLK), /* EDEADLK */
+};
+#undef E
+
+#define E(err) [err - 512 + BUILD_BUG_ON_ZERO(err < 512 || err > 550)] = #err
+static const char *codes_512[] = {
+       E(ERESTARTSYS),
+       E(ERESTARTNOINTR),
+       E(ERESTARTNOHAND),
+       E(ENOIOCTLCMD),
+       E(ERESTART_RESTARTBLOCK),
+       E(EPROBE_DEFER),
+       E(EOPENSTALE),
+       E(ENOPARAM),
+
+       E(EBADHANDLE),
+       E(ENOTSYNC),
+       E(EBADCOOKIE),
+       E(ENOTSUPP),
+       E(ETOOSMALL),
+       E(ESERVERFAULT),
+       E(EBADTYPE),
+       E(EJUKEBOX),
+       E(EIOCBQUEUED),
+       E(ERECALLCONFLICT),
+};
+#undef E
+
+const char *errcode(int err)
+{
+       if (err <= 0)
+               return NULL;
+       if (err < ARRAY_SIZE(codes_0))
+               return codes_0[err];
+       if (err >= 512 && err - 512 < ARRAY_SIZE(codes_512))
+               return codes_512[err - 512];
+       /* But why? */
+       if (IS_ENABLED(CONFIG_MIPS) && err == EDQUOT) /* 1133 */
+               return "EDQUOT";
+       return NULL;
+}
diff --git a/lib/test_printf.c b/lib/test_printf.c
index 944eb50f3862..0401a2341245 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -212,6 +212,7 @@ test_string(void)
 #define PTR_STR "ffff0123456789ab"
 #define PTR_VAL_NO_CRNG "(____ptrval____)"
 #define ZEROS "00000000"       /* hex 32 zero bits */
+#define FFFFS "ffffffff"
 
 static int __init
 plain_format(void)
@@ -243,6 +244,7 @@ plain_format(void)
 #define PTR_STR "456789ab"
 #define PTR_VAL_NO_CRNG "(ptrval)"
 #define ZEROS ""
+#define FFFFS ""
 
 static int __init
 plain_format(void)
@@ -588,6 +590,17 @@ flags(void)
        kfree(cmp_buffer);
 }
 
+static void __init
+errptr(void)
+{
+       test("-1234", "%p", ERR_PTR(-1234));
+       test(FFFFS "ffffffff " FFFFS "ffffff00", "%px %px", ERR_PTR(-1), 
ERR_PTR(-256));
+#ifdef CONFIG_SYMBOLIC_ERRCODE
+       test("EIO EINVAL ENOSPC", "%p %p %p", ERR_PTR(-EIO), ERR_PTR(-EINVAL), 
ERR_PTR(-ENOSPC));
+       test("EAGAIN EAGAIN", "%p %p", ERR_PTR(-EAGAIN), ERR_PTR(-EWOULDBLOCK));
+#endif
+}
+
 static void __init
 test_pointer(void)
 {
@@ -610,6 +623,7 @@ test_pointer(void)
        bitmap();
        netdev_features();
        flags();
+       errptr();
 }
 
 static void __init selftest(void)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index b0967cf17137..299fce317eb3 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -21,6 +21,7 @@
 #include <linux/build_bug.h>
 #include <linux/clk.h>
 #include <linux/clk-provider.h>
+#include <linux/errcode.h>
 #include <linux/module.h>      /* for KSYM_SYMBOL_LEN */
 #include <linux/types.h>
 #include <linux/string.h>
@@ -2111,6 +2112,31 @@ static noinline_for_stack
 char *pointer(const char *fmt, char *buf, char *end, void *ptr,
              struct printf_spec spec)
 {
+       /*
+        * If it's an ERR_PTR, try to print its symbolic
+        * representation, except for %px, where the user explicitly
+        * wanted the pointer formatted as a hex value.
+        */
+       if (IS_ERR(ptr) && *fmt != 'x') {
+               int err = PTR_ERR(ptr);
+               const char *sym = errcode(-err);
+               if (sym)
+                       return string_nocheck(buf, end, sym, spec);
+               /*
+                * Funky, somebody passed ERR_PTR(-1234) or some other
+                * non-existing Efoo - or more likely
+                * CONFIG_SYMBOLIC_ERRCODE=n. None of the
+                * %p<something> extensions can make any sense of an
+                * ERR_PTR(), and if this was just a plain %p, the
+                * user is still better off getting the decimal
+                * representation rather than the hash value that
+                * ptr_to_id() would generate.
+                */
+               spec.flags |= SIGN;
+               spec.base = 10;
+               return number(buf, end, err, spec);
+       }
+
        switch (*fmt) {
        case 'F':
        case 'f':
-- 
2.20.1

Reply via email to