[PATCH v3 0/7] Printk symbol specifier

[PATCH v3 0/7] Printk symbol specifier

[Andrew Cooper]

This series implements %ps and %pS.

Changes from v2:
  Incorperated all comments from the list
  Moved the change for dump_timer() to later in the series
  Remove some redundant _p() casts
  Adjust behaviour of %ps for cases where offset != 0

Changes from v1:
  The first patch has grown to three.  The breakdown is now:

The first two patches refactor string() and pointer() respectivly out of
vsnprintf().  This is just code motion in preparation for the subsequent
patch.

The third patch is the main implementation of %ps and %pS.  With the
refactoring from the first patch, the recursive call to scnprintf in v1 can be
removed.

One inconsistency with the behaviour of print_symbol() is that in the case
that a symbol cant be found, the pointer is printed as per %p, rather than
"???" as before.

Documentation is now provided in docs/misc/printk-formats.txt

The fourth and fifth patches replace all uses of print_symbol() with %ps/%pS,
for x86 and arm respectively.  Most replacements are straight replacements,
but I have taken the opportunity to slightly cleanup the stack tracing code.
When two CPUs are racing at printing a stack, they contend on the console
spinlock, resulting in interleaving across end of the partial strings.  Now,
each full line of the stack trace is printed from a single printk(), so the
interleaving will occur at the line boundaries rather than mid-line
boundaries.

The sixth patch removes print_symbol() and friends, now that the functionality
has been completely replaced.

The seventh patch is not intended for committing, but for people wishing to
test and verify some of the boundary conditions.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Stefano Stabellini <stefano.stabellini@citrix.com>
CC: Tim Deegan <tim@xen.org>

-- 
1.7.10.4

[Patch v3 1/7] common/vsprintf: Refactor string() out of vsnprintf()

[Andrew Cooper]

No functional change

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>
---
 xen/common/vsprintf.c |   49 ++++++++++++++++++++++++++++---------------------
 1 file changed, 28 insertions(+), 21 deletions(-)

diff --git a/xen/common/vsprintf.c b/xen/common/vsprintf.c
index 95bf85d..1bcbd79 100644
--- a/xen/common/vsprintf.c
+++ b/xen/common/vsprintf.c
@@ -235,6 +235,32 @@ static char *number(
     return buf;
 }
 
+static char *string(char *str, char *end, const char *s,
+                    int field_width, int precision, int flags)
+{
+    int i, 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;
+    }
+
+    return str;
+}
+
 /**
  * vsnprintf - Format a string and place it in a buffer
  * @buf: The buffer to place the result into
@@ -255,9 +281,8 @@ static char *number(
  */
 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;
 
@@ -370,25 +395,7 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
             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, s, field_width, precision, flags);
             continue;
 
         case 'p':
-- 
1.7.10.4

[Patch v3 2/7] common/vsprintf: Refactor pointer() out of vsnprintf()

[Andrew Cooper]

No functional change

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>

---

Changes in v3:
 * Pass void* rather than unsigned long
 * Consolidate return
---
 xen/common/vsprintf.c |   23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/xen/common/vsprintf.c b/xen/common/vsprintf.c
index 1bcbd79..8f8d2f6 100644
--- a/xen/common/vsprintf.c
+++ b/xen/common/vsprintf.c
@@ -261,6 +261,20 @@ static char *string(char *str, char *end, const char *s,
     return str;
 }
 
+static char *pointer(char *str, char *end,
+                     const void *arg, int field_width, int precision,
+                     int flags)
+{
+    if ( field_width == -1 )
+    {
+        field_width = 2 * sizeof(void *);
+        flags |= ZEROPAD;
+    }
+
+    return number(str, end, (unsigned long)arg,
+                  16, field_width, precision, flags);
+}
+
 /**
  * vsnprintf - Format a string and place it in a buffer
  * @buf: The buffer to place the result into
@@ -399,13 +413,8 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
             continue;
 
         case 'p':
-            if (field_width == -1) {
-                field_width = 2*sizeof(void *);
-                flags |= ZEROPAD;
-            }
-            str = number(str, end,
-                         (unsigned long) va_arg(args, void *),
-                         16, field_width, precision, flags);
+            str = pointer(str, end, va_arg(args, void *),
+                          field_width, precision, flags);
             continue;
 
 
-- 
1.7.10.4

[Patch v3 3/7] common/vsprintf: Add %ps and %pS format specifier support

[Andrew Cooper]

Introduce the %ps and %pS format options for printing a symbol.

  %ps will print the symbol name and optional offset and size
  %pS will print the symbol name and unconditional offset and size

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>

---

Changes in v3:
 * %ps now only conditionally excludes the offset and size.
 * Coding style
---
 docs/misc/printk-formats.txt |   17 +++++++++++++++++
 xen/common/vsprintf.c        |   42 ++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 57 insertions(+), 2 deletions(-)
 create mode 100644 docs/misc/printk-formats.txt

diff --git a/docs/misc/printk-formats.txt b/docs/misc/printk-formats.txt
new file mode 100644
index 0000000..4c4222b
--- /dev/null
+++ b/docs/misc/printk-formats.txt
@@ -0,0 +1,17 @@
+Xen custom %p format options.  A subset, borrowed from Linux.
+
+All parameters to a %p option should be compatible with void*.  Regular
+pointers are fine.  Numbers should make use of the _p() macro.
+
+Symbol/Function pointers:
+
+       %ps     Symbol name with condition offset and size (iff offset != 0)
+                 e.g.  printk
+                       default_idle+0x78/0x7d
+
+       %pS     Symbol name with unconditional offset and size
+                 e.g.  printk+0/0x48
+                       default_idle+0x78/0x7d
+
+       In the case that an appropriate symbol name can't be found, %p[sS] will
+       fall back to '%p' and print the address in hex.
diff --git a/xen/common/vsprintf.c b/xen/common/vsprintf.c
index 8f8d2f6..f26aa42 100644
--- a/xen/common/vsprintf.c
+++ b/xen/common/vsprintf.c
@@ -17,6 +17,7 @@
  */
 
 #include <xen/ctype.h>
+#include <xen/symbols.h>
 #include <xen/lib.h>
 #include <asm/div64.h>
 #include <asm/page.h>
@@ -261,10 +262,46 @@ static char *string(char *str, char *end, const char *s,
     return str;
 }
 
-static char *pointer(char *str, char *end,
+static char *pointer(char *str, char *end, const char **fmt_ptr,
                      const void *arg, int field_width, int precision,
                      int flags)
 {
+    const char *fmt = *fmt_ptr, *s;
+
+    /* Custom %p suffixes. See XEN_ROOT/docs/misc/printk-formats.txt */
+    switch ( fmt[1] )
+    {
+    case 's': /* Symbol name with offset and size (iff offset != 0) */
+    case 'S': /* Symbol name unconditionally with offset and size */
+    {
+        unsigned long sym_size, sym_offset;
+        char namebuf[KSYM_NAME_LEN+1];
+
+        /* Advance parents fmt string, as we have consumed 's' or 'S' */
+        ++*fmt_ptr;
+
+        s = symbols_lookup((unsigned long)arg, &sym_size, &sym_offset, namebuf);
+
+        /* If the symbol is not found, fall back to printing the address */
+        if ( !s )
+            break;
+
+        /* Print symbol name */
+        str = string(str, end, s, -1, -1, 0);
+
+        if ( fmt[1] == 'S' || sym_offset != 0 )
+        {
+            /* Print '+<offset>/<len>' */
+            str = number(str, end, sym_offset, 16, -1, -1, SPECIAL|SIGN|PLUS);
+            if ( str <= end )
+                *str++ = '/';
+            str = number(str, end, sym_size, 16, -1, -1, SPECIAL);
+        }
+
+        return str;
+    }
+    }
+
     if ( field_width == -1 )
     {
         field_width = 2 * sizeof(void *);
@@ -413,7 +450,8 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
             continue;
 
         case 'p':
-            str = pointer(str, end, va_arg(args, void *),
+            /* pointer() might advance fmt (%pS for example) */
+            str = pointer(str, end, &fmt, va_arg(args, void *),
                           field_width, precision, flags);
             continue;
 
-- 
1.7.10.4

[Patch v3 4/7] x86: Replace print_symbol() with new %ps/%pS format

[Andrew Cooper]

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>

--

Changes in v3:
 * Don't regress printing hpet_broadcast_setup function pointer
 * Move common code to later patch, making this x86 exclusive
 * Remove pointless use of _p() for function pointers
---
 xen/arch/x86/irq.c          |    7 ++-----
 xen/arch/x86/time.c         |    4 ++--
 xen/arch/x86/traps.c        |   30 ++++++++++--------------------
 xen/arch/x86/x86_64/traps.c |    2 +-
 4 files changed, 15 insertions(+), 28 deletions(-)

diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index a984bda..a47d4f6 100644
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -2276,7 +2276,7 @@ static void dump_irqs(unsigned char key)
             printk("\n");
         }
         else if ( desc->action )
-            print_symbol("%s\n", (unsigned long)desc->action->handler);
+            printk("%ps()\n", desc->action->handler);
         else
             printk("mapped, unbound\n");
 
@@ -2288,10 +2288,7 @@ static void dump_irqs(unsigned char key)
     printk("Direct vector information:\n");
     for ( i = FIRST_DYNAMIC_VECTOR; i < NR_VECTORS; ++i )
         if ( direct_apic_vector[i] )
-        {
-            printk("   %#02x -> ", i);
-            print_symbol("%s\n", (unsigned long)direct_apic_vector[i]);
-        }
+            printk("   %#02x -> %ps()\n", i, direct_apic_vector[i]);
 
     dump_ioapic_irq_info();
 }
diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index c1bbd50..21224f0 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -1475,8 +1475,8 @@ static int _disable_pit_irq(void(*hpet_broadcast_setup)(void))
         {
             if ( xen_cpuidle > 0 )
             {
-                print_symbol("%s() failed, turning to PIT broadcast\n",
-                             (unsigned long)hpet_broadcast_setup);
+                printk("%ps() failed, turning to PIT broadcast\n",
+                       hpet_broadcast_setup);
                 return -1;
             }
             ret = 0;
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 77c200b..0e3c6e3 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -198,19 +198,14 @@ static void show_trace(struct cpu_user_regs *regs)
 {
     unsigned long *stack = ESP_BEFORE_EXCEPTION(regs), addr;
 
-    printk("Xen call trace:\n   ");
-
-    printk("[<%p>]", _p(regs->eip));
-    print_symbol(" %s\n   ", regs->eip);
+    printk("Xen call trace:\n"
+           "   [<%p>] %pS\n", _p(regs->eip), _p(regs->eip));
 
     while ( ((long)stack & (STACK_SIZE-BYTES_PER_LONG)) != 0 )
     {
         addr = *stack++;
         if ( is_active_kernel_text(addr) )
-        {
-            printk("[<%p>]", _p(addr));
-            print_symbol(" %s\n   ", addr);
-        }
+            printk("   [<%p>] %pS\n", _p(addr), _p(addr));
     }
 
     printk("\n");
@@ -222,8 +217,6 @@ static void show_trace(struct cpu_user_regs *regs)
 {
     unsigned long *frame, next, addr, low, high;
 
-    printk("Xen call trace:\n   ");
-
     /*
      * If RIP is not pointing into hypervisor code then someone may have
      * called into oblivion. Peek to see if they left a return address at
@@ -232,8 +225,9 @@ static void show_trace(struct cpu_user_regs *regs)
     addr = is_active_kernel_text(regs->eip) ||
            !is_active_kernel_text(*ESP_BEFORE_EXCEPTION(regs)) ?
            regs->eip : *ESP_BEFORE_EXCEPTION(regs);
-    printk("[<%p>]", _p(addr));
-    print_symbol(" %s\n   ", addr);
+
+    printk("Xen call trace:\n"
+           "   [<%p>] %pS\n", _p(addr), _p(addr));
 
     /* Bounds for range of valid frame pointer. */
     low  = (unsigned long)(ESP_BEFORE_EXCEPTION(regs) - 2);
@@ -269,8 +263,7 @@ static void show_trace(struct cpu_user_regs *regs)
             addr  = frame[1];
         }
 
-        printk("[<%p>]", _p(addr));
-        print_symbol(" %s\n   ", addr);
+        printk("   [<%p>] %pS\n", _p(addr), _p(addr));
 
         low = (unsigned long)&frame[2];
     }
@@ -338,10 +331,7 @@ void show_stack_overflow(unsigned int cpu, unsigned long esp)
     {
         addr = *stack++;
         if ( is_active_kernel_text(addr) )
-        {
-            printk("%p: [<%p>]", stack, _p(addr));
-            print_symbol(" %s\n   ", addr);
-        }
+            printk("%p: [<%p>] %pS\n", stack, _p(addr), _p(addr));
     }
 
     printk("\n");
@@ -3755,8 +3745,8 @@ void asm_domain_crash_synchronous(unsigned long addr)
     if ( addr == 0 )
         addr = this_cpu(last_extable_addr);
 
-    printk("domain_crash_sync called from entry.S: fault at %p ", _p(addr));
-    print_symbol("%s\n", addr);
+    printk("domain_crash_sync called from entry.S: fault at %p %pS\n",
+           _p(addr), _p(addr));
 
     __domain_crash_synchronous();
 }
diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c
index 0316d7c..ae93539 100644
--- a/xen/arch/x86/x86_64/traps.c
+++ b/xen/arch/x86/x86_64/traps.c
@@ -49,7 +49,7 @@ static void _show_registers(
 
     printk("RIP:    %04x:[<%016lx>]", regs->cs, regs->rip);
     if ( context == CTXT_hypervisor )
-        print_symbol(" %s", regs->rip);
+        printk(" %pS", _p(regs->rip));
     printk("\nRFLAGS: %016lx   ", regs->rflags);
     if ( (context == CTXT_pv_guest) && v && v->vcpu_info )
         printk("EM: %d   ", !!vcpu_info(v, evtchn_upcall_mask));
-- 
1.7.10.4

[Patch v3 5/7] arm: Replace print_symbol() with new %ps/%pS format

[Andrew Cooper]

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
CC: Stefano Stabellini <stefano.stabellini@citrix.com>
CC: Tim Deegan <tim@xen.org>
---
 xen/arch/arm/traps.c |   15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 287dd7b..10ee498 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -387,7 +387,7 @@ static void show_registers_32(struct cpu_user_regs *regs,
 #else
     printk("PC:     %08"PRIx32, regs->pc);
     if ( !guest_mode )
-        print_symbol(" %s", regs->pc);
+        printk(" %pS", _p(regs->pc));
     printk("\n");
 #endif
     printk("CPSR:   %08"PRIx32" MODE:%s\n", regs->cpsr,
@@ -462,7 +462,7 @@ static void show_registers_64(struct cpu_user_regs *regs,
 
     printk("PC:     %016"PRIx64, regs->pc);
     if ( !guest_mode )
-        print_symbol(" %s", regs->pc);
+        printk(" %pS", _p(regs->pc));
     printk("\n");
     printk("LR:     %016"PRIx64"\n", regs->lr);
     if ( guest_mode )
@@ -735,12 +735,10 @@ static void show_trace(struct cpu_user_regs *regs)
 {
     register_t *frame, next, addr, low, high;
 
-    printk("Xen call trace:\n   ");
+    printk("Xen call trace:\n");
 
-    printk("[<%p>]", _p(regs->pc));
-    print_symbol(" %s (PC)\n   ", regs->pc);
-    printk("[<%p>]", _p(regs->lr));
-    print_symbol(" %s (LR)\n   ", regs->lr);
+    printk("   [<%p>] %pS (PC)\n", _p(regs->pc), _p(regs->pc));
+    printk("   [<%p>] %pS (LR)\n", _p(regs->lr), _p(regs->lr));
 
     /* Bounds for range of valid frame pointer. */
     low  = (register_t)(STACK_BEFORE_EXCEPTION(regs));
@@ -760,8 +758,7 @@ static void show_trace(struct cpu_user_regs *regs)
         next  = frame[0];
         addr  = frame[1];
 
-        printk("[<%p>]", _p(addr));
-        print_symbol(" %s\n   ", addr);
+        printk("   [<%p>] %pS\n", _p(addr), _p(addr));
 
         low = (register_t)&frame[1];
     }
-- 
1.7.10.4

[Patch v3 6/7] common/symbols: Remove print_symbol() and associated infrastructure

[Andrew Cooper]

Also adjust the one common user of print_symbol() to use the new printk()
format.  While adjusting the format string, increase the width so a
long-to-expire plt_overflow() timer doesn't break the column alignment.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>

---

Changes in v3:
 * Move the single common change from x86 patch to here
---
 xen/common/symbols.c      |   26 --------------------------
 xen/common/timer.c        |    3 +--
 xen/include/xen/symbols.h |   23 -----------------------
 3 files changed, 1 insertion(+), 51 deletions(-)

diff --git a/xen/common/symbols.c b/xen/common/symbols.c
index 83b2b58..45941e1 100644
--- a/xen/common/symbols.c
+++ b/xen/common/symbols.c
@@ -148,29 +148,3 @@ const char *symbols_lookup(unsigned long addr,
     *offset = addr - symbols_address(low);
     return namebuf;
 }
-
-/* Replace "%s" in format with address, or returns -errno. */
-void __print_symbol(const char *fmt, unsigned long address)
-{
-    const char *name;
-    unsigned long offset, size, flags;
-
-    static DEFINE_SPINLOCK(lock);
-    static char namebuf[KSYM_NAME_LEN+1];
-#define BUFFER_SIZE sizeof("%s+%#lx/%#lx [%s]") + KSYM_NAME_LEN + \
-			2*(BITS_PER_LONG*3/10) + 1
-    static char buffer[BUFFER_SIZE];
-
-    spin_lock_irqsave(&lock, flags);
-
-    name = symbols_lookup(address, &size, &offset, namebuf);
-
-    if (!name)
-        snprintf(buffer, BUFFER_SIZE, "???");
-    else
-        snprintf(buffer, BUFFER_SIZE, "%s+%#lx/%#lx", name, offset, size);
-
-    printk(fmt, buffer);
-
-    spin_unlock_irqrestore(&lock, flags);
-}
diff --git a/xen/common/timer.c b/xen/common/timer.c
index 9ed74e9..1895a78 100644
--- a/xen/common/timer.c
+++ b/xen/common/timer.c
@@ -511,9 +511,8 @@ s_time_t align_timer(s_time_t firsttick, uint64_t period)
 
 static void dump_timer(struct timer *t, s_time_t now)
 {
-    printk("  ex=%8"PRId64"us timer=%p cb=%p(%p)",
+    printk("  ex=%12"PRId64"us timer=%p cb=%ps(%p)\n",
            (t->expires - now) / 1000, t, t->function, t->data);
-    print_symbol(" %s\n", (unsigned long)t->function);
 }
 
 static void dump_timerq(unsigned char key)
diff --git a/xen/include/xen/symbols.h b/xen/include/xen/symbols.h
index 37cf6bf..87cd77d 100644
--- a/xen/include/xen/symbols.h
+++ b/xen/include/xen/symbols.h
@@ -11,27 +11,4 @@ const char *symbols_lookup(unsigned long addr,
                            unsigned long *offset,
                            char *namebuf);
 
-/* Replace "%s" in format with address, if found */
-void __print_symbol(const char *fmt, unsigned long address);
-
-/* This macro allows us to keep printk typechecking */
-static void __check_printsym_format(const char *fmt, ...)
-    __attribute__((format(printf,1,2)));
-    static inline void __check_printsym_format(const char *fmt, ...)
-{
-}
-
-#if 0
-#define print_fn_descriptor_symbol(fmt, addr)	\
-	print_symbol(fmt, *(unsigned long *)addr)
-#else
-#define print_fn_descriptor_symbol(fmt, addr) print_symbol(fmt, addr)
-#endif
-
-#define print_symbol(fmt, addr)			\
-do {						\
-	__check_printsym_format(fmt, "");	\
-	__print_symbol(fmt, addr);		\
-} while(0)
-
 #endif /*_XEN_SYMBOLS_H*/
-- 
1.7.10.4

[Patch v3 7/7] Test harness for new printk formatting

[Andrew Cooper]

Not intended for committing
---
 xen/arch/x86/irq.c |   60 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index a47d4f6..046230f 100644
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -2485,3 +2485,63 @@ bool_t hvm_domain_use_pirq(const struct domain *d, const struct pirq *pirq)
     return is_hvm_domain(d) && pirq &&
            pirq->arch.hvm.emuirq != IRQ_UNBOUND; 
 }
+
+static void test_printk(unsigned char key)
+{
+    char buffer[28] = { ' ' };
+    unsigned i, t;
+
+    printk("'%c' pressed - In %s()\n", key, __func__);
+    printk("Some symbols: '%ps' '%pS'\n", _p(printk), _p(printk));
+    printk("More symbols: '%ps' '%pS'\n", _p(test_printk), _p(test_printk));
+    printk("Invalid: '%ps' '%pS'\n", _p(0x20), _p(0x20));
+    printk("Partial: '%ps' '%pS'\n", _p(((unsigned long)test_printk + 0x20)),
+           _p(((unsigned long)test_printk + 0x20)));
+
+    /* Use 's as guards */
+    buffer[0] = '\''; buffer[sizeof buffer - 1] = '\0';
+
+    for ( i = 6; i < sizeof(buffer) - 2; ++i )
+    {
+        t = snprintf(&buffer[1], i, "%pS", _p(test_printk));
+
+        if ( buffer[i] == '\0' )
+            buffer[i] = '\'';
+
+        printk("Test %.2u byte buffer %s\n", i, buffer);
+
+        if ( t < i )
+            break;
+    }
+
+    memset(&buffer[1], ' ', sizeof buffer - 2);
+    buffer[sizeof buffer - 1] = '\0';
+
+    for ( i = 6; i < sizeof(buffer) - 2; ++i )
+    {
+        t = snprintf(&buffer[1], i, "%ps", _p(((unsigned long)test_printk + 0x20)));
+
+        if ( buffer[i] == '\0' )
+            buffer[i] = '\'';
+
+        printk("Test %.2u byte buffer %s\n", i, buffer);
+
+        if ( t < i )
+            break;
+    }
+
+    printk("Testing pointer: '%ps' '%pS'\n", dom0, dom0);
+}
+
+static struct keyhandler test_printk_keyhandler = {
+    .diagnostic = 1,
+    .u.fn = test_printk,
+    .desc = "Test new printk formatting options"
+};
+
+static int __init setup_test_printk(void)
+{
+    register_keyhandler('6', &test_printk_keyhandler);
+    return 0;
+}
+__initcall(setup_test_printk);
-- 
1.7.10.4

Re: [PATCH v3 0/7] Printk symbol specifier

[Jan Beulich]

>>> On 05.11.13 at 15:38, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> This series implements %ps and %pS.
> 
> Changes from v2:
>   Incorperated all comments from the list
>   Moved the change for dump_timer() to later in the series
>   Remove some redundant _p() casts
>   Adjust behaviour of %ps for cases where offset != 0

Looks all good to me now, but 1-3 and 6 will - as usual - require
Keir's approval.

Jan

Re: [PATCH v3 0/7] Printk symbol specifier

[Andrew Cooper]

On 05/11/13 15:04, Jan Beulich wrote:
>>>> On 05.11.13 at 15:38, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> This series implements %ps and %pS.
>>
>> Changes from v2:
>>   Incorperated all comments from the list
>>   Moved the change for dump_timer() to later in the series
>>   Remove some redundant _p() casts
>>   Adjust behaviour of %ps for cases where offset != 0
> Looks all good to me now, but 1-3 and 6 will - as usual - require
> Keir's approval.
>
> Jan

Keir: ping  (As you appear to have a review cycle on other patches, but
perhaps missed this series?)

~Andrew

> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH v3 0/7] Printk symbol specifier

[Keir Fraser]

On 06/11/2013 17:48, "Andrew Cooper" <andrew.cooper3@citrix.com> wrote:

> On 05/11/13 15:04, Jan Beulich wrote:
>>>>> On 05.11.13 at 15:38, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>>> This series implements %ps and %pS.
>>> 
>>> Changes from v2:
>>>   Incorperated all comments from the list
>>>   Moved the change for dump_timer() to later in the series
>>>   Remove some redundant _p() casts
>>>   Adjust behaviour of %ps for cases where offset != 0
>> Looks all good to me now, but 1-3 and 6 will - as usual - require
>> Keir's approval.
>> 
>> Jan
> 
> Keir: ping  (As you appear to have a review cycle on other patches, but
> perhaps missed this series?)

Sorry, look good to me:
Acked-by: Keir Fraser <keir@xen.org>

> ~Andrew
> 
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel