Messages in this thread | | | Subject | Re: [RFC PATCH] vsprintf: add flag ZEROPAD handling before crng is ready | From | Andy Shevchenko <> | Date | Fri, 26 Jan 2018 11:17:56 +0200 |
| |
+Rasmus
On Fri, 2018-01-26 at 15:39 +0800, Yang Shunyong wrote: > Before crng is ready, output of "%p" composes of "(ptrval)" and > left padding spaces for alignment as no random address can be > generated. This seems a little strange sometimes. > For example, when irq domain names are built with "%p", the nodes > under /sys/kernel/debug/irq/domains like this, > > [root@y irq]# ls domains/ > default irqchip@ (ptrval)-2 > irqchip@ (ptrval)-4 \_SB_.TCS0.QIC1 \_SB_.TCS0.QIC3 > irqchip@ (ptrval) irqchip@ (ptrval)-3 > \_SB_.TCS0.QIC0 \_SB_.TCS0.QIC2 > > The name "irqchip@ (ptrval)-2" is not so readable in console > output.
Yes, this is not best output.
> This patch adds ZEROPAD handling in widen_string() and move_right(). > When ZEROPAD is set in spec, it will use '0' for padding. If not > set, it will use ' '. > This patch also sets ZEROPAD in ptr_to_id() before crgn is ready.
Did you run tests?
Have you added specific test cases to see what's going on for patterns like
printf("%0s\n", " (my string)");
?
> Following is the output after applying the patch, > > [root@y irq]# ls domains/ > default irqchip@00000000(ptrval)-2 > irqchip@00000000(ptrval)-4 \_SB_.TCS0.QIC1 \_SB_.TCS0.QIC3 > irqchip@00000000(ptrval) irqchip@00000000(ptrval)-3 \_SB_.TCS0.QIC0 > \_SB_.TCS0.QIC2 >
So, for me it looks like curing symptoms. After all, it's debugfs, no one prevents you to fix an output of the certain nodes there.
> I am not sure whether crng can get enough random data at very early > stage of system startup (eg. before irq system in the case above) > and the effort to port current random driver to work at that time. > So, this issue may exist some time. > I use '0' for padding in this patch. This should be ok because output > of "%pK" is all '0's when kptr_restrict = 2. I don't know whether > other character, such as 'x', may be more preferable. > > Signed-off-by: Yang Shunyong <shunyong.yang@hxt-semitech.com> > --- > lib/vsprintf.c | 19 ++++++++++++++----- > 1 file changed, 14 insertions(+), 5 deletions(-) > > diff --git a/lib/vsprintf.c b/lib/vsprintf.c > index 01c3957b2de6..e0b6e1edae31 100644 > --- a/lib/vsprintf.c > +++ b/lib/vsprintf.c > @@ -535,14 +535,18 @@ char *special_hex_number(char *buf, char *end, > unsigned long long num, int size) > return number(buf, end, num, spec); > } > > -static void move_right(char *buf, char *end, unsigned len, unsigned > spaces) > +static void move_right(char *buf, char *end, unsigned int len, > + unsigned int spaces, struct printf_spec spec) > { > size_t size; > + char pad; > + > + pad = (spec.flags & ZEROPAD) ? '0' : ' '; > if (buf >= end) /* nowhere to put anything */ > return; > size = end - buf; > if (size <= spaces) { > - memset(buf, ' ', size); > + memset(buf, pad, size); > return; > } > if (len) { > @@ -550,7 +554,7 @@ static void move_right(char *buf, char *end, > unsigned len, unsigned spaces) > len = size - spaces; > memmove(buf + spaces, buf, len); > } > - memset(buf, ' ', spaces); > + memset(buf, pad, spaces); > } > > /* > @@ -565,18 +569,21 @@ static void move_right(char *buf, char *end, > unsigned len, unsigned spaces) > char *widen_string(char *buf, int n, char *end, struct printf_spec > spec) > { > unsigned spaces; > + char pad; > > if (likely(n >= spec.field_width)) > return buf; > /* we want to pad the sucker */ > spaces = spec.field_width - n; > if (!(spec.flags & LEFT)) { > - move_right(buf - n, end, n, spaces); > + move_right(buf - n, end, n, spaces, spec); > return buf + spaces; > } > + > + pad = (spec.flags & ZEROPAD) ? '0' : ' '; > while (spaces--) { > if (buf < end) > - *buf = ' '; > + *buf = pad; > ++buf; > } > return buf; > @@ -1702,6 +1709,8 @@ static char *ptr_to_id(char *buf, char *end, > void *ptr, struct printf_spec spec) > > if (unlikely(!have_filled_random_ptr_key)) { > spec.field_width = default_width; > + spec.flags |= ZEROPAD; > + > /* string length must be less than default_width */ > return string(buf, end, "(ptrval)", spec); > }
-- Andy Shevchenko <andriy.shevchenko@linux.intel.com> Intel Finland Oy
| |