Messages in this thread | | | Date | Sat, 28 Feb 2009 18:18:39 +0100 | From | Frederic Weisbecker <> | Subject | Re: [PATCH][RFC] vsprintf: unify the format decoding layer for its 3 users |
| |
On Sat, Feb 28, 2009 at 08:54:15AM -0800, Linus Torvalds wrote: > > > 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
Ok, I'm addressing your comments and those from Ingo, then I will rebase the whole patchset (with the ftrace_bprintk related things) against latest -tip and resend it.
Thanks!
| |