Messages in this thread | | | Date | Sat, 28 Feb 2009 08:54:15 -0800 (PST) | From | Linus Torvalds <> | Subject | Re: [PATCH][RFC] vsprintf: unify the format decoding layer for its 3 users |
| |
On Sat, 28 Feb 2009, Frederic Weisbecker wrote: > > You're right, that's much proper. > See the V2 below:
Good. This looks better.
One final (?) nit:
> + default: { > + enum format_type type = spec.type; > + > + if (type == FORMAT_TYPE_LONG_LONG) > + num = va_arg(args, long long); > + else if (type == FORMAT_TYPE_ULONG) > + num = va_arg(args, unsigned long); > + else if (type == FORMAT_TYPE_LONG) > + num = va_arg(args, unsigned long); > + else if (type == FORMAT_TYPE_SIZE_T) > + num = va_arg(args, size_t); > + else if (type == FORMAT_TYPE_PTRDIFF) > + num = va_arg(args, ptrdiff_t); > + else if (type == FORMAT_TYPE_USHORT) > + num = (unsigned short) va_arg(args, int); > + else if (type == FORMAT_TYPE_SHORT) > + num = (short) va_arg(args, int); > + else if (type == FORMAT_TYPE_UINT) > + num = va_arg(args, unsigned int); > + else > + num = va_arg(args, unsigned int); > + > + str = number(str, end, num, spec.base, > + spec.field_width, spec.precision, spec.flags);
You've got three copes of these (in the three different users), and that just looks horrid. I see why you did it, but even something like
case FORMAT_TYPE_LONG_LONG: num = va_arg(args, long long); goto handle_number; case FORMAT_TYPE_ULONG: ... case FORMAT_TYPE_INT: num = va_arg(args, int); handle_number: str = number(str, end, num, &spec); break;
would be better.
And yes, please do pass in "spec" to the "number()/pointer()/string()" routines instead of exploding it into the individual numbers. When cleaning up, let's just do it properly..
Linus
| |