Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1283341 > unrolled thread

[PATCH v3 00/14] printf stuff for 4.5

Back to article view | Back to linux.kernel

Contents

  [PATCH v3 00/14] printf stuff for 4.5 Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2015-12-03 22:00 +0100
    [PATCH v3 01/14] lib/vsprintf.c: pull out padding code from dentry_name() Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2015-12-03 22:00 +0100
    Re: [PATCH v3 04/14] lib/vsprintf.c: expand field_width to 24 bits Joe Perches <joe@perches.com> - 2015-12-03 22:00 +0100
      Re: [PATCH v3 04/14] lib/vsprintf.c: expand field_width to 24 bits Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2015-12-03 22:40 +0100
        Re: [PATCH v3 04/14] lib/vsprintf.c: expand field_width to 24 bits Andrew Morton <akpm@linux-foundation.org> - 2015-12-04 00:40 +0100
          Re: [PATCH v3 04/14] lib/vsprintf.c: expand field_width to 24 bits Joe Perches <joe@perches.com> - 2015-12-04 01:10 +0100
            Re: [PATCH v3 04/14] lib/vsprintf.c: expand field_width to 24 bits Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2015-12-04 10:10 +0100
          Re: [PATCH v3 04/14] lib/vsprintf.c: expand field_width to 24 bits Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2015-12-04 10:10 +0100
        Re: [PATCH v3 04/14] lib/vsprintf.c: expand field_width to 24 bits Joe Perches <joe@perches.com> - 2015-12-04 00:50 +0100
    [PATCH v3 08/14] lib/test_printf.c: don't BUG Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2015-12-03 22:00 +0100
    [PATCH v3 03/14] lib/vsprintf.c: eliminate potential race in string() Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2015-12-03 22:00 +0100
    [PATCH v3 06/14] lib/vsprintf.c: warn about too large precisions and field widths Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2015-12-03 22:00 +0100
    [PATCH v3 09/14] lib/test_printf.c: check for out-of-bound writes Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2015-12-03 22:00 +0100
    [PATCH v3 02/14] lib/vsprintf.c: move string() below widen_string() Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2015-12-03 22:00 +0100
    [PATCH v3 14/14] lib/test_printf.c: test dentry printing Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2015-12-03 22:00 +0100
      Re: [PATCH v3 14/14] lib/test_printf.c: test dentry printing Andrew Morton <akpm@linux-foundation.org> - 2015-12-04 01:20 +0100
        Re: [PATCH v3 14/14] lib/test_printf.c: test dentry printing Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2015-12-04 09:20 +0100
          Re: [PATCH v3 14/14] lib/test_printf.c: test dentry printing Andrew Morton <akpm@linux-foundation.org> - 2015-12-04 09:50 +0100
    [PATCH v3 04/14] lib/vsprintf.c: expand field_width to 24 bits Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2015-12-03 22:00 +0100

#1283341 — [PATCH v3 00/14] printf stuff for 4.5

I was too late for 4.4, so here's hoping to get this into 4.5.

1-3 fix a theoretical race in printing strings via %s - since we're
reusing existing code from the dentry printer, we actually also win on
code size. Ingo, can I perhaps get you to turn your "looks good to"
into an ack?

4 fixes a problem introduced by converting all bitmap formatting to go
via %pb[l] - it turns out that bitmaps with >= 1<<15 bits are actually
printed sometimes. This isn't necessarily the best fix, but the
discussion died out, so I'm proposing this for now.

5 is just a minor optimization (maybe not so much on
register-challenged arches).

6 I'm not too sure about, but maybe the bitmap problem had been
discovered sooner (it took a little over half a year to be reported)
if these had been in place.

Paranoid-me wanted me to include patch 7, but I don't have strong
feelings for it.

8-14 are minor additions to the test module (some with acks from
Kees).


Rasmus Villemoes (14):
  lib/vsprintf.c: pull out padding code from dentry_name()
  lib/vsprintf.c: move string() below widen_string()
  lib/vsprintf.c: eliminate potential race in string()
  lib/vsprintf.c: expand field_width to 24 bits
  lib/vsprintf.c: help gcc make number() smaller
  lib/vsprintf.c: warn about too large precisions and field widths
  lib/kasprintf.c: add sanity check to kvasprintf
  lib/test_printf.c: don't BUG
  lib/test_printf.c: check for out-of-bound writes
  lib/test_printf.c: test precision quirks
  lib/test_printf.c: add a few number() tests
  lib/test_printf.c: account for kvasprintf tests
  lib/test_printf.c: add test for large bitmaps
  lib/test_printf.c: test dentry printing

 lib/kasprintf.c   |  10 +--
 lib/test_printf.c | 121 ++++++++++++++++++++++++++++++----
 lib/vsprintf.c    | 193 +++++++++++++++++++++++++++++++-----------------------
 3 files changed, 226 insertions(+), 98 deletions(-)

-- 
2.6.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1283342 — [PATCH v3 01/14] lib/vsprintf.c: pull out padding code from dentry_name()

Pull out the logic in dentry_name() which handles field width space
padding, in preparation for reusing it from string(). Rename the
widen() helper to move_right(), since it is used for handling the
!(flags & LEFT) case.

Cc: Al Viro <viro@ZenIV.linux.org.uk>
Cc: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 lib/vsprintf.c | 46 +++++++++++++++++++++++++++++++---------------
 1 file changed, 31 insertions(+), 15 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index f9cee8e1233c..d7452563a6a6 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -538,7 +538,7 @@ char *string(char *buf, char *end, const char *s, struct printf_spec spec)
 	return buf;
 }
 
-static void widen(char *buf, char *end, unsigned len, unsigned spaces)
+static void move_right(char *buf, char *end, unsigned len, unsigned spaces)
 {
 	size_t size;
 	if (buf >= end)	/* nowhere to put anything */
@@ -556,6 +556,35 @@ static void widen(char *buf, char *end, unsigned len, unsigned spaces)
 	memset(buf, ' ', spaces);
 }
 
+/*
+ * Handle field width padding for a string.
+ * @buf: current buffer position
+ * @n: length of string
+ * @end: end of output buffer
+ * @spec: for field width and flags
+ * Returns: new buffer position after padding.
+ */
+static noinline_for_stack
+char *widen_string(char *buf, int n, char *end, struct printf_spec spec)
+{
+	unsigned spaces;
+
+	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);
+		return buf + spaces;
+	}
+	while (spaces--) {
+		if (buf < end)
+			*buf = ' ';
+		++buf;
+	}
+	return buf;
+}
+
 static noinline_for_stack
 char *dentry_name(char *buf, char *end, const struct dentry *d, struct printf_spec spec,
 		  const char *fmt)
@@ -597,20 +626,7 @@ char *dentry_name(char *buf, char *end, const struct dentry *d, struct printf_sp
 			*buf = c;
 	}
 	rcu_read_unlock();
-	if (n < spec.field_width) {
-		/* we want to pad the sucker */
-		unsigned spaces = spec.field_width - n;
-		if (!(spec.flags & LEFT)) {
-			widen(buf - n, end, n, spaces);
-			return buf + spaces;
-		}
-		while (spaces--) {
-			if (buf < end)
-				*buf = ' ';
-			++buf;
-		}
-	}
-	return buf;
+	return widen_string(buf, n, end, spec);
 }
 
 static noinline_for_stack
-- 
2.6.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1283345 — Re: [PATCH v3 04/14] lib/vsprintf.c: expand field_width to 24 bits

On Thu, 2015-12-03 at 21:51 +0100, Rasmus Villemoes wrote:
> Maurizio Lombardi reported a problem [1] with the %pb extension: It
> doesn't work for sufficiently large bitmaps, since the size is stashed
> in the field_width field of the struct printf_spec, which is currently
> an s16. Concretely, this manifested itself in
> /sys/bus/pseudo/drivers/scsi_debug/map being empty, since the bitmap
> printer got a size of 0, which is the 16 bit truncation of the actual
> bitmap size.
> 
> We do want to keep struct printf_spec at 8 bytes so that it can
> cheaply be passed by value. The qualifier field is only used for
> internal bookkeeping in format_decode, so we might as well use a local
> variable for that. This gives us an additional 8 bits, which we can
> then use for the field width.
> 
> To stay in 8 bytes, we need to do a little rearranging and make the
> type member a bitfield as well. For consistency, change all the
> members to bit fields. gcc doesn't generate much worse code with these
> changes (in fact, bloat-o-meter says we save 300 bytes - which I think
> is a little surprising).
> 
> I didn't find a BUILD_BUG/compiletime_assertion/... which would work
> outside function context, so for now I just open-coded it.
> 
> [1] http://thread.gmane.org/gmane.linux.kernel/2034835

Thanks for keeping at this Rasmus.
This seems quite reasonable.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1283404 — Re: [PATCH v3 04/14] lib/vsprintf.c: expand field_width to 24 bits

On Thu, 2015-12-03 at 12:54 -0800, Joe Perches wrote:
> On Thu, 2015-12-03 at 21:51 +0100, Rasmus Villemoes wrote:
> > Maurizio Lombardi reported a problem [1] with the %pb extension: It
> > doesn't work for sufficiently large bitmaps, since the size is
> > stashed
> > in the field_width field of the struct printf_spec, which is
> > currently
> > an s16. Concretely, this manifested itself in
> > /sys/bus/pseudo/drivers/scsi_debug/map being empty, since the
> > bitmap
> > printer got a size of 0, which is the 16 bit truncation of the
> > actual
> > bitmap size.
> > 
> > We do want to keep struct printf_spec at 8 bytes so that it can
> > cheaply be passed by value. The qualifier field is only used for
> > internal bookkeeping in format_decode, so we might as well use a
> > local
> > variable for that. This gives us an additional 8 bits, which we can
> > then use for the field width.
> > 
> > To stay in 8 bytes, we need to do a little rearranging and make the
> > type member a bitfield as well. For consistency, change all the
> > members to bit fields. gcc doesn't generate much worse code with
> > these
> > changes (in fact, bloat-o-meter says we save 300 bytes - which I
> > think
> > is a little surprising).
> > 
> > I didn't find a BUILD_BUG/compiletime_assertion/... which would
> > work
> > outside function context, so for now I just open-coded it.
> > 
> > [1] http://thread.gmane.org/gmane.linux.kernel/2034835
> 
> Thanks for keeping at this Rasmus.
> This seems quite reasonable.

I like most of the stuff here, though, Joe, can we avoid open-coded
BUILD_BUG_ON()?


-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1283456 — Re: [PATCH v3 04/14] lib/vsprintf.c: expand field_width to 24 bits

On Thu, 03 Dec 2015 23:28:58 +0200 Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Thu, 2015-12-03 at 12:54 -0800, Joe Perches wrote:
> > On Thu, 2015-12-03 at 21:51 +0100, Rasmus Villemoes wrote:
> > > Maurizio Lombardi reported a problem [1] with the %pb extension: It
> > > doesn't work for sufficiently large bitmaps, since the size is
> > > stashed
> > > in the field_width field of the struct printf_spec, which is
> > > currently
> > > an s16. Concretely, this manifested itself in
> > > /sys/bus/pseudo/drivers/scsi_debug/map being empty, since the
> > > bitmap
> > > printer got a size of 0, which is the 16 bit truncation of the
> > > actual
> > > bitmap size.
> > > 
> > > We do want to keep struct printf_spec at 8 bytes so that it can
> > > cheaply be passed by value. The qualifier field is only used for
> > > internal bookkeeping in format_decode, so we might as well use a
> > > local
> > > variable for that. This gives us an additional 8 bits, which we can
> > > then use for the field width.
> > > 
> > > To stay in 8 bytes, we need to do a little rearranging and make the
> > > type member a bitfield as well. For consistency, change all the
> > > members to bit fields. gcc doesn't generate much worse code with
> > > these
> > > changes (in fact, bloat-o-meter says we save 300 bytes - which I
> > > think
> > > is a little surprising).
> > > 
> > > I didn't find a BUILD_BUG/compiletime_assertion/... which would
> > > work
> > > outside function context, so for now I just open-coded it.
> > > 
> > > [1] http://thread.gmane.org/gmane.linux.kernel/2034835
> > 
> > Thanks for keeping at this Rasmus.
> > This seems quite reasonable.
> 
> I like most of the stuff here, though, Joe, can we avoid open-coded
> BUILD_BUG_ON()?

Well we could just do

--- a/lib/vsprintf.c~lib-vsprintfc-expand-field_width-to-24-bits-fix
+++ a/lib/vsprintf.c
@@ -386,7 +386,6 @@ struct printf_spec {
 	unsigned int	base:8;		/* number base, 8, 10 or 16 only */
 	signed int	precision:16;	/* # of digits/chars */
 } __packed;
-extern char __check_printf_spec[1-2*(sizeof(struct printf_spec) != 8)];
 
 static noinline_for_stack
 char *number(char *buf, char *end, unsigned long long num,
@@ -400,6 +399,8 @@ char *number(char *buf, char *end, unsig
 	int i;
 	bool is_zero = num == 0LL;
 
+	BUILD_BUG_ON(sizeof(struct printf_spec) != 8);
+
 	/* locase = 0 or 0x20. ORing digits or letters with 'locase'
 	 * produces same digits or (maybe lowercased) letters */
 	locase = (spec.flags & SMALL);

Which is better than open-coding it, IMO.



I've been fiddling with a BUILD_BUG_ON which works outside functions
using gcc's __COUNTER__ - something like

#define BBO(expr) typedef char __bbo##__COUNTER__[1-2*(!!expr)]

BBO(1 == 1);
BBO(2 == 2);

but that comes out as

typedef char __bbo__COUNTER__[1-2*(!!1 == 1)];
typedef char __bbo__COUNTER__[1-2*(!!2 == 2)];

instead of

typedef char __bbo0[1-2*(!!1 == 1)];
typedef char __bbo1[1-2*(!!2 == 2)];

There's some trick here but I've forgotten what it is.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1283466 — Re: [PATCH v3 04/14] lib/vsprintf.c: expand field_width to 24 bits

On Thu, 2015-12-03 at 15:34 -0800, Andrew Morton wrote:
> I've been fiddling with a BUILD_BUG_ON which works outside functions
> using gcc's __COUNTER__ - something like
> 
> #define BBO(expr) typedef char __bbo##__COUNTER__[1-2*(!!expr)]

nit:  you need another parenthesis around expr

> BBO(1 == 1);
> BBO(2 == 2);
> 
> but that comes out as
> 
> typedef char __bbo__COUNTER__[1-2*(!!1 == 1)];
> typedef char __bbo__COUNTER__[1-2*(!!2 == 2)];
> 
> instead of
> 
> typedef char __bbo0[1-2*(!!1 == 1)];
> typedef char __bbo1[1-2*(!!2 == 2)];
> 
> There's some trick here but I've forgotten what it is.

I believe it's something like:

#define __stringify_2(a, b)	a##b
#define __stringify2(a, b)	__stringify_2(a, b)

#define BBO(expr) typedef char __stringify2(bbo, __COUNTER__)[1 - 2*(!!(expr))]
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1283659 — Re: [PATCH v3 04/14] lib/vsprintf.c: expand field_width to 24 bits

On Fri, Dec 04 2015, Joe Perches <joe@perches.com> wrote:

> On Thu, 2015-12-03 at 15:34 -0800, Andrew Morton wrote:
>> I've been fiddling with a BUILD_BUG_ON which works outside functions
>> using gcc's __COUNTER__ - something like
>> 
>> #define BBO(expr) typedef char __bbo##__COUNTER__[1-2*(!!expr)]
>
> nit:  you need another parenthesis around expr
>
>> BBO(1 == 1);
>> BBO(2 == 2);
>> 
>> but that comes out as
>> 
>> typedef char __bbo__COUNTER__[1-2*(!!1 == 1)];
>> typedef char __bbo__COUNTER__[1-2*(!!2 == 2)];
>> 
>> instead of
>> 
>> typedef char __bbo0[1-2*(!!1 == 1)];
>> typedef char __bbo1[1-2*(!!2 == 2)];
>> 
>> There's some trick here but I've forgotten what it is.
>
> I believe it's something like:
>
> #define __stringify_2(a, b)	a##b
> #define __stringify2(a, b)	__stringify_2(a, b)
>
> #define BBO(expr) typedef char __stringify2(bbo, __COUNTER__)[1 - 2*(!!(expr))]

Let's at least not reinvent two wheels. __UNIQUE_ID exists and does the
gluing (which we have __PASTE for, not stringify) etc., and uses
__LINE__ as a poor man's fallback for compilers without __COUNTER__ (gcc
< 4.3).

But I don't see why we even need the unique identifier. What's wrong
with 'extern char blabla[1 - 2*(!!(expr))]'? blabla can be declared
multiple times without problems - and when it fails, we either get a
'negative size' error or at least some complaint about conflicting
declarations. Maybe stick a __always_unused in to prevent gcc from
complaining if this declaration is inside a function.

Rasmus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1283662 — Re: [PATCH v3 04/14] lib/vsprintf.c: expand field_width to 24 bits

On Fri, Dec 04 2015, Andrew Morton <akpm@linux-foundation.org> wrote:

> On Thu, 03 Dec 2015 23:28:58 +0200 Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>
>> I like most of the stuff here, though, Joe, can we avoid open-coded
>> BUILD_BUG_ON()?
>
> Well we could just do
>
[snip]
>
> Which is better than open-coding it, IMO.

I'd really prefer to have the assertion near the type definition (which
is why I did it that way), but if this is what it takes to get this
patch in, fine by me.

Rasmus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1283458 — Re: [PATCH v3 04/14] lib/vsprintf.c: expand field_width to 24 bits

On Thu, 2015-12-03 at 23:28 +0200, Andy Shevchenko wrote:
> On Thu, 2015-12-03 at 12:54 -0800, Joe Perches wrote:
> > On Thu, 2015-12-03 at 21:51 +0100, Rasmus Villemoes wrote:
[]
> > > I didn't find a BUILD_BUG/compiletime_assertion/... which would work
> > > outside function context, so for now I just open-coded it.
> > > 
> > > [1] http://thread.gmane.org/gmane.linux.kernel/2034835
[]
> I like most of the stuff here, though, can we avoid open-coded
> BUILD_BUG_ON()?

Not so far as I can know.  Maybe another generic could be added.

It doesn't seem this specific check is all that useful.

sizeof(struct printf_spec) != 8

I suppose there could be an assert in some function instead.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1283347 — [PATCH v3 08/14] lib/test_printf.c: don't BUG

BUG is a completely unnecessarily big hammer, and we're more likely to
get the internal bug reported if we just pr_err() and ensure the test
suite fails.

Acked-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 lib/test_printf.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/lib/test_printf.c b/lib/test_printf.c
index c5a666af9ba5..9232a2add28c 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -91,7 +91,12 @@ __test(const char *expect, int elen, const char *fmt, ...)
 	int rand;
 	char *p;
 
-	BUG_ON(elen >= BUF_SIZE);
+	if (elen >= BUF_SIZE) {
+		pr_err("error in test suite: expected output length %d too long. Format was '%s'.\n",
+		       elen, fmt);
+		failed_tests++;
+		return;
+	}
 
 	va_start(ap, fmt);
 
-- 
2.6.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1283348 — [PATCH v3 03/14] lib/vsprintf.c: eliminate potential race in string()

If the string corresponding to a %s specifier can change under us, we
might end up copying a \0 byte to the output buffer. There might be
callers who expect the output buffer to contain a genuine C string
whose length is exactly the snprintf return value (assuming truncation
hasn't happened or has been checked for).

We can avoid this by only passing over the source string once,
stopping the first time we meet a nul byte (or when we reach the given
precision), and then letting widen_string() handle left/right space
padding. As a small bonus, this code reuse also makes the generated
code slightly smaller.

Cc: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 lib/vsprintf.c | 28 +++++++++-------------------
 1 file changed, 9 insertions(+), 19 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index a021e6380404..63ca52366049 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -557,32 +557,22 @@ char *widen_string(char *buf, int n, char *end, struct printf_spec spec)
 static noinline_for_stack
 char *string(char *buf, char *end, const char *s, struct printf_spec spec)
 {
-	int len, i;
+	int len = 0;
+	size_t lim = spec.precision;
 
 	if ((unsigned long)s < PAGE_SIZE)
 		s = "(null)";
 
-	len = strnlen(s, spec.precision);
-
-	if (!(spec.flags & LEFT)) {
-		while (len < spec.field_width--) {
-			if (buf < end)
-				*buf = ' ';
-			++buf;
-		}
-	}
-	for (i = 0; i < len; ++i) {
-		if (buf < end)
-			*buf = *s;
-		++buf; ++s;
-	}
-	while (len < spec.field_width--) {
+	while (lim--) {
+		char c = *s++;
+		if (!c)
+			break;
 		if (buf < end)
-			*buf = ' ';
+			*buf = c;
 		++buf;
+		++len;
 	}
-
-	return buf;
+	return widen_string(buf, len, end, spec);
 }
 
 static noinline_for_stack
-- 
2.6.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1283349 — [PATCH v3 06/14] lib/vsprintf.c: warn about too large precisions and field widths

The field width is overloaded to pass some extra information for
some %p extensions (e.g. #bits for %pb). But we might silently
truncate the passed value when we stash it in struct printf_spec (see
e.g. "lib/vsprintf.c: expand field_width to 24 bits").  Hopefully 23
value bits should now be enough for everybody, but if not, let's make
some noise.

Do the same for the precision. In both cases, clamping seems more
sensible than truncating. While, according to POSIX, "A negative
precision is taken as if the precision were omitted.", the kernel's
printf has always treated that case as if the precision was 0, so we
use that as lower bound. For the field width, the smallest
representable value is actually -(1<<23), but a negative field width
means 'set the LEFT flag and use the absolute value', so we want the
absolute value to fit.

Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
v3: also do the check in bstr_printf (thanks Andy).

 lib/vsprintf.c | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index d7e27c54fa00..3db2281e6026 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -386,6 +386,8 @@ struct printf_spec {
 	unsigned int	base:8;		/* number base, 8, 10 or 16 only */
 	signed int	precision:16;	/* # of digits/chars */
 } __packed;
+#define FIELD_WIDTH_MAX ((1 << 23) - 1)
+#define PRECISION_MAX ((1 << 15) - 1)
 extern char __check_printf_spec[1-2*(sizeof(struct printf_spec) != 8)];
 
 static noinline_for_stack
@@ -1815,6 +1817,24 @@ qualifier:
 	return ++fmt - start;
 }
 
+static void
+set_field_width(struct printf_spec *spec, int width)
+{
+	spec->field_width = width;
+	if (WARN_ONCE(spec->field_width != width, "field width %d too large", width)) {
+		spec->field_width = clamp(width, -FIELD_WIDTH_MAX, FIELD_WIDTH_MAX);
+	}
+}
+
+static void
+set_precision(struct printf_spec *spec, int prec)
+{
+	spec->precision = prec;
+	if (WARN_ONCE(spec->precision != prec, "precision %d too large", prec)) {
+		spec->precision = clamp(prec, 0, PRECISION_MAX);
+	}
+}
+
 /**
  * vsnprintf - Format a string and place it in a buffer
  * @buf: The buffer to place the result into
@@ -1882,11 +1902,11 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
 		}
 
 		case FORMAT_TYPE_WIDTH:
-			spec.field_width = va_arg(args, int);
+			set_field_width(&spec, va_arg(args, int));
 			break;
 
 		case FORMAT_TYPE_PRECISION:
-			spec.precision = va_arg(args, int);
+			set_precision(&spec, va_arg(args, int));
 			break;
 
 		case FORMAT_TYPE_CHAR: {
@@ -2326,11 +2346,11 @@ int bstr_printf(char *buf, size_t size, const char *fmt, const u32 *bin_buf)
 		}
 
 		case FORMAT_TYPE_WIDTH:
-			spec.field_width = get_arg(int);
+			set_field_width(&spec, get_arg(int));
 			break;
 
 		case FORMAT_TYPE_PRECISION:
-			spec.precision = get_arg(int);
+			set_precision(&spec, get_arg(int));
 			break;
 
 		case FORMAT_TYPE_CHAR: {
-- 
2.6.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1283350 — [PATCH v3 09/14] lib/test_printf.c: check for out-of-bound writes

Add a few padding bytes on either side of the test buffer, and check
that these (and the part of the buffer not used) are untouched by
vsnprintf.

Acked-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 lib/test_printf.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/lib/test_printf.c b/lib/test_printf.c
index 9232a2add28c..1ce1a1dd8faf 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -16,6 +16,7 @@
 #include <linux/in.h>
 
 #define BUF_SIZE 256
+#define PAD_SIZE 16
 #define FILL_CHAR '$'
 
 #define PTR1 ((void*)0x01234567)
@@ -39,6 +40,7 @@
 static unsigned total_tests __initdata;
 static unsigned failed_tests __initdata;
 static char *test_buffer __initdata;
+static char *alloced_buffer __initdata;
 
 static int __printf(4, 0) __init
 do_test(int bufsize, const char *expect, int elen,
@@ -49,7 +51,7 @@ do_test(int bufsize, const char *expect, int elen,
 
 	total_tests++;
 
-	memset(test_buffer, FILL_CHAR, BUF_SIZE);
+	memset(alloced_buffer, FILL_CHAR, BUF_SIZE + 2*PAD_SIZE);
 	va_copy(aq, ap);
 	ret = vsnprintf(test_buffer, bufsize, fmt, aq);
 	va_end(aq);
@@ -60,8 +62,13 @@ do_test(int bufsize, const char *expect, int elen,
 		return 1;
 	}
 
+	if (memchr_inv(alloced_buffer, FILL_CHAR, PAD_SIZE)) {
+		pr_warn("vsnprintf(buf, %d, \"%s\", ...) wrote before buffer\n", bufsize, fmt);
+		return 1;
+	}
+
 	if (!bufsize) {
-		if (memchr_inv(test_buffer, FILL_CHAR, BUF_SIZE)) {
+		if (memchr_inv(test_buffer, FILL_CHAR, BUF_SIZE + PAD_SIZE)) {
 			pr_warn("vsnprintf(buf, 0, \"%s\", ...) wrote to buffer\n",
 				fmt);
 			return 1;
@@ -76,6 +83,12 @@ do_test(int bufsize, const char *expect, int elen,
 		return 1;
 	}
 
+	if (memchr_inv(test_buffer + written + 1, FILL_CHAR, BUF_SIZE + PAD_SIZE - (written + 1))) {
+		pr_warn("vsnprintf(buf, %d, \"%s\", ...) wrote beyond the nul-terminator\n",
+			bufsize, fmt);
+		return 1;
+	}
+
 	if (memcmp(test_buffer, expect, written)) {
 		pr_warn("vsnprintf(buf, %d, \"%s\", ...) wrote '%s', expected '%.*s'\n",
 			bufsize, fmt, test_buffer, written, expect);
@@ -342,16 +355,17 @@ test_pointer(void)
 static int __init
 test_printf_init(void)
 {
-	test_buffer = kmalloc(BUF_SIZE, GFP_KERNEL);
-	if (!test_buffer)
+	alloced_buffer = kmalloc(BUF_SIZE + 2*PAD_SIZE, GFP_KERNEL);
+	if (!alloced_buffer)
 		return -ENOMEM;
+	test_buffer = alloced_buffer + PAD_SIZE;
 
 	test_basic();
 	test_number();
 	test_string();
 	test_pointer();
 
-	kfree(test_buffer);
+	kfree(alloced_buffer);
 
 	if (failed_tests == 0)
 		pr_info("all %u tests passed\n", total_tests);
-- 
2.6.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1283351 — [PATCH v3 02/14] lib/vsprintf.c: move string() below widen_string()

This is pure code movement, making sure the widen_string() helper is
defined before the string() function.

Cc: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 lib/vsprintf.c | 62 +++++++++++++++++++++++++++++-----------------------------
 1 file changed, 31 insertions(+), 31 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index d7452563a6a6..a021e6380404 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -507,37 +507,6 @@ char *number(char *buf, char *end, unsigned long long num,
 	return buf;
 }
 
-static noinline_for_stack
-char *string(char *buf, char *end, const char *s, struct printf_spec spec)
-{
-	int len, i;
-
-	if ((unsigned long)s < PAGE_SIZE)
-		s = "(null)";
-
-	len = strnlen(s, spec.precision);
-
-	if (!(spec.flags & LEFT)) {
-		while (len < spec.field_width--) {
-			if (buf < end)
-				*buf = ' ';
-			++buf;
-		}
-	}
-	for (i = 0; i < len; ++i) {
-		if (buf < end)
-			*buf = *s;
-		++buf; ++s;
-	}
-	while (len < spec.field_width--) {
-		if (buf < end)
-			*buf = ' ';
-		++buf;
-	}
-
-	return buf;
-}
-
 static void move_right(char *buf, char *end, unsigned len, unsigned spaces)
 {
 	size_t size;
@@ -586,6 +555,37 @@ char *widen_string(char *buf, int n, char *end, struct printf_spec spec)
 }
 
 static noinline_for_stack
+char *string(char *buf, char *end, const char *s, struct printf_spec spec)
+{
+	int len, i;
+
+	if ((unsigned long)s < PAGE_SIZE)
+		s = "(null)";
+
+	len = strnlen(s, spec.precision);
+
+	if (!(spec.flags & LEFT)) {
+		while (len < spec.field_width--) {
+			if (buf < end)
+				*buf = ' ';
+			++buf;
+		}
+	}
+	for (i = 0; i < len; ++i) {
+		if (buf < end)
+			*buf = *s;
+		++buf; ++s;
+	}
+	while (len < spec.field_width--) {
+		if (buf < end)
+			*buf = ' ';
+		++buf;
+	}
+
+	return buf;
+}
+
+static noinline_for_stack
 char *dentry_name(char *buf, char *end, const struct dentry *d, struct printf_spec spec,
 		  const char *fmt)
 {
-- 
2.6.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1283352 — [PATCH v3 14/14] lib/test_printf.c: test dentry printing

Cc: Al Viro <viro@ZenIV.linux.org.uk>
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 lib/test_printf.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/lib/test_printf.c b/lib/test_printf.c
index 60740c10c3e8..0234356c6698 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -13,6 +13,7 @@
 #include <linux/string.h>
 
 #include <linux/bitmap.h>
+#include <linux/dcache.h>
 #include <linux/socket.h>
 #include <linux/in.h>
 
@@ -326,9 +327,35 @@ uuid(void)
 	test("03020100-0504-0706-0809-0A0B0C0D0E0F", "%pUL", uuid);
 }
 
+static struct dentry test_dentry[4] __initdata = {
+	{ .d_parent = &test_dentry[0],
+	  .d_name = { .len = 3, .name = test_dentry[0].d_iname },
+	  .d_iname = "foo" },
+	{ .d_parent = &test_dentry[0],
+	  .d_name = { .len = 5, .name = test_dentry[1].d_iname },
+	  .d_iname = "bravo" },
+	{ .d_parent = &test_dentry[1],
+	  .d_name = { .len = 4, .name = test_dentry[2].d_iname },
+	  .d_iname = "alfa" },
+	{ .d_parent = &test_dentry[2],
+	  .d_name = { .len = 5, .name = test_dentry[3].d_iname },
+	  .d_iname = "romeo" },
+};
+
 static void __init
 dentry(void)
 {
+	test("foo", "%pd", &test_dentry[0]);
+	test("foo", "%pd2", &test_dentry[0]);
+
+	test("romeo", "%pd", &test_dentry[3]);
+	test("alfa/romeo", "%pd2", &test_dentry[3]);
+	test("bravo/alfa/romeo", "%pd3", &test_dentry[3]);
+	test("/bravo/alfa/romeo", "%pd4", &test_dentry[3]);
+	test("/bravo/alfa", "%pd4", &test_dentry[2]);
+
+	test("bravo/alfa  |bravo/alfa  ", "%-12pd2|%*pd2", &test_dentry[2], -12, &test_dentry[2]);
+	test("  bravo/alfa|  bravo/alfa", "%12pd2|%*pd2", &test_dentry[2], 12, &test_dentry[2]);
 }
 
 static void __init
-- 
2.6.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1283474 — Re: [PATCH v3 14/14] lib/test_printf.c: test dentry printing

On Thu,  3 Dec 2015 21:51:13 +0100 Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote:

> +static struct dentry test_dentry[4] __initdata = {
> +	{ .d_parent = &test_dentry[0],
> +	  .d_name = { .len = 3, .name = test_dentry[0].d_iname },
> +	  .d_iname = "foo" },

Confused.  qstr has no .len.

lib/test_printf.c:332: error: unknown field 'len' specified in initializer

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1283624 — Re: [PATCH v3 14/14] lib/test_printf.c: test dentry printing

On Fri, Dec 04 2015, Andrew Morton <akpm@linux-foundation.org> wrote:

> On Thu,  3 Dec 2015 21:51:13 +0100 Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote:
>
>> +static struct dentry test_dentry[4] __initdata = {
>> +	{ .d_parent = &test_dentry[0],
>> +	  .d_name = { .len = 3, .name = test_dentry[0].d_iname },
>> +	  .d_iname = "foo" },
>
> Confused.  qstr has no .len.
>
> lib/test_printf.c:332: error: unknown field 'len' specified in initializer

Huh? It goes without saying that I've compiled and run this, and never
seen this problem. struct qstr does have a len member, though it's
hidden inside an anonymous struct inside an anonymous union. But at
least my gcc (4.9) has no problem with those initializers - and
fs/dcache.c happily accesses name->len all the time (how else would one
get at the member). Also note the lack of 0day complaints. What tool
gave that error?

Also confused.

Rasmus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1283648 — Re: [PATCH v3 14/14] lib/test_printf.c: test dentry printing

On Fri, 04 Dec 2015 09:16:02 +0100 Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote:

> On Fri, Dec 04 2015, Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > On Thu,  3 Dec 2015 21:51:13 +0100 Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote:
> >
> >> +static struct dentry test_dentry[4] __initdata = {
> >> +	{ .d_parent = &test_dentry[0],
> >> +	  .d_name = { .len = 3, .name = test_dentry[0].d_iname },
> >> +	  .d_iname = "foo" },
> >
> > Confused.  qstr has no .len.
> >
> > lib/test_printf.c:332: error: unknown field 'len' specified in initializer
> 
> Huh? It goes without saying that I've compiled and run this, and never
> seen this problem. struct qstr does have a len member, though it's
> hidden inside an anonymous struct inside an anonymous union. But at
> least my gcc (4.9) has no problem with those initializers - and
> fs/dcache.c happily accesses name->len all the time (how else would one
> get at the member). Also note the lack of 0day complaints. What tool
> gave that error?
> 

Ah, OK, that's the gcc-4.4.4 bug with initialization of anonymous
unions.

I really should get a new compiler, but running old compilers has value
- it finds problems with old compilers!

I've never found a workaround for this problem so the fix thus far has
been to do the initialization in regular old C code.

However it appears that the QSTR_INIT() macro (which you should have
used anyway!) contains some magic sauce.  This fixes it:

--- a/lib/test_printf.c~lib-test_printfc-test-dentry-printing-fix
+++ a/lib/test_printf.c
@@ -329,16 +329,16 @@ uuid(void)
 
 static struct dentry test_dentry[4] __initdata = {
 	{ .d_parent = &test_dentry[0],
-	  .d_name = { .len = 3, .name = test_dentry[0].d_iname },
+	  .d_name = QSTR_INIT(test_dentry[0].d_iname, 3),
 	  .d_iname = "foo" },
 	{ .d_parent = &test_dentry[0],
-	  .d_name = { .len = 5, .name = test_dentry[1].d_iname },
+	  .d_name = QSTR_INIT(test_dentry[1].d_iname, 5),
 	  .d_iname = "bravo" },
 	{ .d_parent = &test_dentry[1],
-	  .d_name = { .len = 4, .name = test_dentry[2].d_iname },
+	  .d_name = QSTR_INIT(test_dentry[2].d_iname, 4),
 	  .d_iname = "alfa" },
 	{ .d_parent = &test_dentry[2],
-	  .d_name = { .len = 5, .name = test_dentry[3].d_iname },
+	  .d_name = QSTR_INIT(test_dentry[3].d_iname, 5),
 	  .d_iname = "romeo" },
 };
 
I assume the code still works ;)



And while I'm there...

From: Andrew Morton <akpm@linux-foundation.org>
Subject: include/linux/dcache.h: remove semicolons from HASH_LEN_DECLARE

A little cleanup - the invocation site provdes the semicolon.

Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Al Viro <viro@ZenIV.linux.org.uk>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 include/linux/dcache.h |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff -puN include/linux/dcache.h~include-linux-dcacheh-remove-semicolons-from-hash_len_declare include/linux/dcache.h
--- a/include/linux/dcache.h~include-linux-dcacheh-remove-semicolons-from-hash_len_declare
+++ a/include/linux/dcache.h
@@ -27,10 +27,10 @@ struct vfsmount;
 
 /* The hash is always the low bits of hash_len */
 #ifdef __LITTLE_ENDIAN
- #define HASH_LEN_DECLARE u32 hash; u32 len;
+ #define HASH_LEN_DECLARE u32 hash; u32 len
  #define bytemask_from_count(cnt)	(~(~0ul << (cnt)*8))
 #else
- #define HASH_LEN_DECLARE u32 len; u32 hash;
+ #define HASH_LEN_DECLARE u32 len; u32 hash
  #define bytemask_from_count(cnt)	(~(~0ul >> (cnt)*8))
 #endif
 
_

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1283353 — [PATCH v3 04/14] lib/vsprintf.c: expand field_width to 24 bits

Maurizio Lombardi reported a problem [1] with the %pb extension: It
doesn't work for sufficiently large bitmaps, since the size is stashed
in the field_width field of the struct printf_spec, which is currently
an s16. Concretely, this manifested itself in
/sys/bus/pseudo/drivers/scsi_debug/map being empty, since the bitmap
printer got a size of 0, which is the 16 bit truncation of the actual
bitmap size.

We do want to keep struct printf_spec at 8 bytes so that it can
cheaply be passed by value. The qualifier field is only used for
internal bookkeeping in format_decode, so we might as well use a local
variable for that. This gives us an additional 8 bits, which we can
then use for the field width.

To stay in 8 bytes, we need to do a little rearranging and make the
type member a bitfield as well. For consistency, change all the
members to bit fields. gcc doesn't generate much worse code with these
changes (in fact, bloat-o-meter says we save 300 bytes - which I think
is a little surprising).

I didn't find a BUILD_BUG/compiletime_assertion/... which would work
outside function context, so for now I just open-coded it.

[1] http://thread.gmane.org/gmane.linux.kernel/2034835

Reported-by: Maurizio Lombardi <mlombard@redhat.com>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Joe Perches <joe@perches.com>
Acked-by: Tejun Heo <tj@kernel.org>
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 lib/vsprintf.c | 41 +++++++++++++++++++++--------------------
 1 file changed, 21 insertions(+), 20 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 63ca52366049..01c3aa638582 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -380,13 +380,13 @@ enum format_type {
 };
 
 struct printf_spec {
-	u8	type;		/* format_type enum */
-	u8	flags;		/* flags to number() */
-	u8	base;		/* number base, 8, 10 or 16 only */
-	u8	qualifier;	/* number qualifier, one of 'hHlLtzZ' */
-	s16	field_width;	/* width of output field */
-	s16	precision;	/* # of digits/chars */
-};
+	unsigned int	type:8;		/* format_type enum */
+	signed int	field_width:24;	/* width of output field */
+	unsigned int	flags:8;	/* flags to number() */
+	unsigned int	base:8;		/* number base, 8, 10 or 16 only */
+	signed int	precision:16;	/* # of digits/chars */
+} __packed;
+extern char __check_printf_spec[1-2*(sizeof(struct printf_spec) != 8)];
 
 static noinline_for_stack
 char *number(char *buf, char *end, unsigned long long num,
@@ -1641,6 +1641,7 @@ static noinline_for_stack
 int format_decode(const char *fmt, struct printf_spec *spec)
 {
 	const char *start = fmt;
+	char qualifier;
 
 	/* we finished early by reading the field width */
 	if (spec->type == FORMAT_TYPE_WIDTH) {
@@ -1723,16 +1724,16 @@ precision:
 
 qualifier:
 	/* get the conversion qualifier */
-	spec->qualifier = -1;
+	qualifier = 0;
 	if (*fmt == 'h' || _tolower(*fmt) == 'l' ||
 	    _tolower(*fmt) == 'z' || *fmt == 't') {
-		spec->qualifier = *fmt++;
-		if (unlikely(spec->qualifier == *fmt)) {
-			if (spec->qualifier == 'l') {
-				spec->qualifier = 'L';
+		qualifier = *fmt++;
+		if (unlikely(qualifier == *fmt)) {
+			if (qualifier == 'l') {
+				qualifier = 'L';
 				++fmt;
-			} else if (spec->qualifier == 'h') {
-				spec->qualifier = 'H';
+			} else if (qualifier == 'h') {
+				qualifier = 'H';
 				++fmt;
 			}
 		}
@@ -1789,19 +1790,19 @@ qualifier:
 		return fmt - start;
 	}
 
-	if (spec->qualifier == 'L')
+	if (qualifier == 'L')
 		spec->type = FORMAT_TYPE_LONG_LONG;
-	else if (spec->qualifier == 'l') {
+	else if (qualifier == 'l') {
 		BUILD_BUG_ON(FORMAT_TYPE_ULONG + SIGN != FORMAT_TYPE_LONG);
 		spec->type = FORMAT_TYPE_ULONG + (spec->flags & SIGN);
-	} else if (_tolower(spec->qualifier) == 'z') {
+	} else if (_tolower(qualifier) == 'z') {
 		spec->type = FORMAT_TYPE_SIZE_T;
-	} else if (spec->qualifier == 't') {
+	} else if (qualifier == 't') {
 		spec->type = FORMAT_TYPE_PTRDIFF;
-	} else if (spec->qualifier == 'H') {
+	} else if (qualifier == 'H') {
 		BUILD_BUG_ON(FORMAT_TYPE_UBYTE + SIGN != FORMAT_TYPE_BYTE);
 		spec->type = FORMAT_TYPE_UBYTE + (spec->flags & SIGN);
-	} else if (spec->qualifier == 'h') {
+	} else if (qualifier == 'h') {
 		BUILD_BUG_ON(FORMAT_TYPE_USHORT + SIGN != FORMAT_TYPE_SHORT);
 		spec->type = FORMAT_TYPE_USHORT + (spec->flags & SIGN);
 	} else {
-- 
2.6.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web