tip.git regression from "vsprintf: unify the format decoding layer for its 3 users"

tip.git regression from "vsprintf: unify the format decoding layer for its 3 users"

[Jeremy Fitzhardinge]

Change fef20d9c1380f04ba9492d6463148db07b413708, "vsprintf: unify the 
format decoding layer for its 3 users", causes a regression in xenbus 
which results in no devices getting attached to a new domain.  Reverting 
fef20d9c1380f04ba9492d6463148db07b413708 and 
39e874f8afbdb3745e2406ce4ecbde9ac4cbaa78 fixes the problem.

I haven't identified what format string is being handled wrongly, so I 
don't know what the precise bug is.  The most complex looking format in 
use seems to be %.*s; there's also "%s/%s", "%i" and "%lX".

    J

Re: tip.git regression from "vsprintf: unify the format decoding layer for its 3 users"

[Vegard Nossum]

2009/3/14 Jeremy Fitzhardinge <jeremy@goop.org>:
> Change fef20d9c1380f04ba9492d6463148db07b413708, "vsprintf: unify the format
> decoding layer for its 3 users", causes a regression in xenbus which results
> in no devices getting attached to a new domain.  Reverting
> fef20d9c1380f04ba9492d6463148db07b413708 and
> 39e874f8afbdb3745e2406ce4ecbde9ac4cbaa78 fixes the problem.
>
> I haven't identified what format string is being handled wrongly, so I don't
> know what the precise bug is.  The most complex looking format in use seems
> to be %.*s; there's also "%s/%s", "%i" and "%lX".

Hi,

At least %.*s seems to be broken. How about this patch?

Vegard


diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index dc16743..be3001f 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -398,7 +398,7 @@ static noinline char* put_dec(char *buf, unsigned long long num)
 
 enum format_type {
 	FORMAT_TYPE_NONE, /* Just a string part */
-	FORMAT_TYPE_WITDH,
+	FORMAT_TYPE_WIDTH,
 	FORMAT_TYPE_PRECISION,
 	FORMAT_TYPE_CHAR,
 	FORMAT_TYPE_STR,
@@ -770,7 +770,7 @@ static int format_decode(const char *fmt, struct printf_spec *spec)
 	const char *start = fmt;
 
 	/* we finished early by reading the field width */
-	if (spec->type == FORMAT_TYPE_WITDH) {
+	if (spec->type == FORMAT_TYPE_WIDTH) {
 		if (spec->field_width < 0) {
 			spec->field_width = -spec->field_width;
 			spec->flags |= LEFT;
@@ -828,7 +828,7 @@ static int format_decode(const char *fmt, struct printf_spec *spec)
 		spec->field_width = skip_atoi(&fmt);
 	else if (*fmt == '*') {
 		/* it's the next argument */
-		spec->type = FORMAT_TYPE_WITDH;
+		spec->type = FORMAT_TYPE_WIDTH;
 		return ++fmt - start;
 	}
 
@@ -843,7 +843,7 @@ precision:
 				spec->precision = 0;
 		} else if (*fmt == '*') {
 			/* it's the next argument */
-			spec->type = FORMAT_TYPE_WITDH;
+			spec->type = FORMAT_TYPE_PRECISION;
 			return ++fmt - start;
 		}
 	}
@@ -1002,7 +1002,7 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
 			break;
 		}
 
-		case FORMAT_TYPE_WITDH:
+		case FORMAT_TYPE_WIDTH:
 			spec.field_width = va_arg(args, int);
 			break;
 
@@ -1306,7 +1306,7 @@ do {									\
 		case FORMAT_TYPE_NONE:
 			break;
 
-		case FORMAT_TYPE_WITDH:
+		case FORMAT_TYPE_WIDTH:
 		case FORMAT_TYPE_PRECISION:
 			save_arg(int);
 			break;
@@ -1472,7 +1472,7 @@ int bstr_printf(char *buf, size_t size, const char *fmt, const u32 *bin_buf)
 			break;
 		}
 
-		case FORMAT_TYPE_WITDH:
+		case FORMAT_TYPE_WIDTH:
 			spec.field_width = get_arg(int);
 			break;
 

Re: tip.git regression from "vsprintf: unify the format decoding layer for its 3 users"

[Jeremy Fitzhardinge]

Vegard Nossum wrote:
> 2009/3/14 Jeremy Fitzhardinge <jeremy@goop.org>:
>   
>> Change fef20d9c1380f04ba9492d6463148db07b413708, "vsprintf: unify the format
>> decoding layer for its 3 users", causes a regression in xenbus which results
>> in no devices getting attached to a new domain.  Reverting
>> fef20d9c1380f04ba9492d6463148db07b413708 and
>> 39e874f8afbdb3745e2406ce4ecbde9ac4cbaa78 fixes the problem.
>>
>> I haven't identified what format string is being handled wrongly, so I don't
>> know what the precise bug is.  The most complex looking format in use seems
>> to be %.*s; there's also "%s/%s", "%i" and "%lX".
>>     
>
> Hi,
>
> At least %.*s seems to be broken. How about this patch?
>   

Thanks, that does the trick.

    J