the printk problem

[Linus Torvalds]

On Fri, 4 Jul 2008, Linus Torvalds wrote:
> 
> so I think we could easily just say that we extend %p in various ways:
> 
>  - %pS - print pointer as a symbol
> 
> and leave tons of room for future extensions for different kinds of 
> pointers. 

So here's a totally untested example patch of this, which could probably 
easily be extended to to other things.

I actually made it '%pF' and '%pS' for a Function descriptor pointer and 
normal Symbolic pointer respectively, because of the stupid things ia64 
and PPC64 do with the pointer indirection through function descriptors. 

That function descriptor indirection is totally untested, and I did it 
with a

	pagefault_disable();
	__get_user(..)
	pagefault_enable();

thing because I thought it would be nice if printk() was always safe, and 
passing bogus function pointers to '%pF' should try to work, but quite 
frankly, I didn't even check that that part compiles, much less works.

ia64/ppc lists cc'd, just in case somebody wants to test this.

NOTE! There are no current actual users of this, but the _idea_ is that we 
should be able to just do

	printk("%pF\n", desc->handle_irq);

instead of using

	print_symbol("%s\n", (unsigned long)desc->handle_irq);

The latter is from kernel/irq/internals.h, and actually looks incorrect - 
shouldn't it use print_fn_descriptor_symbol(), since it's a C level 
function pointer? We should use "print_symbol()" for return pointers we 
find on the stack and data pointers, but not for stuff that actually has a 
C type that is a function pointer?

Somebody who cares about the insane function descriptors should take a 
deeper look.

NOTE AGAIN! UNTESTED! I could easily have screwed up printk() _entirely_, 
since I had to factor out the string handling into a function of its own.

		Linus
---
 lib/vsprintf.c |  102 ++++++++++++++++++++++++++++++++++++++++---------------
 1 files changed, 74 insertions(+), 28 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 6021757..148b656 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -22,6 +22,7 @@
 #include <linux/string.h>
 #include <linux/ctype.h>
 #include <linux/kernel.h>
+#include <linux/kallsyms.h>
 
 #include <asm/page.h>		/* for PAGE_SIZE */
 #include <asm/div64.h>
@@ -482,6 +483,72 @@ static char *number(char *buf, char *end, unsigned long long num, int base, int
 	return buf;
 }
 
+static char *string(char *buf, char *end, char *s, int field_width, int precision, int flags)
+{
+	int len, i;
+
+	if ((unsigned long)s < PAGE_SIZE)
+		s = "<NULL>";
+
+	len = strnlen(s, precision);
+
+	if (!(flags & LEFT)) {
+		while (len < field_width--) {
+			if (buf < end)
+				*buf = ' ';
+			++buf;
+		}
+	}
+	for (i = 0; i < len; ++i) {
+		if (buf < end)
+			*buf = *s;
+		++buf; ++s;
+	}
+	while (len < field_width--) {
+		if (buf < end)
+			*buf = ' ';
+		++buf;
+	}
+	return buf;
+}
+
+/*
+ * Show a '%p' thing.  A kernel extension is that the '%p' is followed
+ * by an extra set of alphanumeric characters that are extended format
+ * specifiers.  Right now we just handle 'F' (for symbolic Function
+ * pointers) and 'S' (for Symbolic data pointers), but this can easily
+ * be extended in the future (network address types etc).
+ *
+ * The difference between 'S' and 'F' is that on ia64 and ppc64 function
+ * pointers are really function descriptors, which contain a pointer the
+ * real address. 
+ */
+static char *pointer(const char *fmt, char *buf, char *end, void *ptr, int base, int size, int precision, int type)
+{
+	char sym[KSYM_SYMBOL_LEN];
+	switch (*fmt) {
+	case 'F':	/* Function pointer */
+#if defined(CONFIG_IA64) || defined(CONFIG_PPC64)
+		{ void *p;
+		pagefault_disable();
+		if (!__get_user((void **)ptr, &p))
+			ptr = p;
+		pagefault_enable();
+		}
+#endif
+	/* Fallthrough */
+	case 'S':	/* Other pointer */
+#if CONFIG_KALLSYMS
+		sprint_symbol(sym, (unsigned long) ptr);
+		return string(buf, end, sym, size, precision, type);
+#else
+		type |= SPECIAL;
+		break;
+#endif
+	}
+	return number(buf, end, (unsigned long long) ptr, base, size, precision, type);
+}
+
 /**
  * vsnprintf - Format a string and place it in a buffer
  * @buf: The buffer to place the result into
@@ -502,11 +569,9 @@ static char *number(char *buf, char *end, unsigned long long num, int base, int
  */
 int vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
 {
-	int len;
 	unsigned long long num;
-	int i, base;
+	int base;
 	char *str, *end, c;
-	const char *s;
 
 	int flags;		/* flags to number() */
 
@@ -622,29 +687,7 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
 				continue;
 
 			case 's':
-				s = va_arg(args, char *);
-				if ((unsigned long)s < PAGE_SIZE)
-					s = "<NULL>";
-
-				len = strnlen(s, precision);
-
-				if (!(flags & LEFT)) {
-					while (len < field_width--) {
-						if (str < end)
-							*str = ' ';
-						++str;
-					}
-				}
-				for (i = 0; i < len; ++i) {
-					if (str < end)
-						*str = *s;
-					++str; ++s;
-				}
-				while (len < field_width--) {
-					if (str < end)
-						*str = ' ';
-					++str;
-				}
+				str = string(str, end, va_arg(args, char *), field_width, precision, flags);
 				continue;
 
 			case 'p':
@@ -653,9 +696,12 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
 					field_width = 2*sizeof(void *);
 					flags |= ZEROPAD;
 				}
-				str = number(str, end,
-						(unsigned long) va_arg(args, void *),
+				str = pointer(fmt+1, str, end,
+						va_arg(args, void *),
 						16, field_width, precision, flags);
+				/* Skip all alphanumeric pointer suffixes */
+				while (isalnum(fmt[1]))
+					fmt++;
 				continue;