On Thu, 15 May 2008, Andrew Morton wrote:
(yeah, I normally parenthesise sizeof too, but this provided 80-col salvation)
Why? The nice way to provide 80-col salvation is to just fix the code. Instead of having complex nested loops, have a really trivial outer loop like for (call = __initcall_start; call < __initcall_end; call++) do_one_initcall(*call); and then the code to actually do one of the initcalls is much simpler to read, and because it's not as indented, the 80-column issues just go away. IOW, something like this (the diff looks ugly, but the end result is better). Talking about cleanups, why the *hell* does print_fn_descriptor_symbol() take an "unsigned long", when every single user would want to give it a pointer? Would somebody please want to move that cast into the macro (or better yet, make it an inline function that takes a 'void *'), and remove all the casts from the callers? Linus --- init/main.c | 78 +++++++++++++++++++++++++++++++---------------------------- 1 files changed, 41 insertions(+), 37 deletions(-) diff --git a/init/main.c b/init/main.c index f406fef..0a1d65e 100644 --- a/init/main.c +++ b/init/main.c @@ -695,53 +695,57 @@ __setup("initcall_debug", initcall_debug_setup); extern initcall_t __initcall_start[], __initcall_end[]; -static void __init do_initcalls(void) +static void __init do_one_initcall(initcall_t call) { - initcall_t *call; int count = preempt_count(); + ktime_t t0, t1, delta; + char msgbuf[64]; + int result; + + if (initcall_debug) { + print_fn_descriptor_symbol("calling %s()\n", + (unsigned long) call); + t0 = ktime_get(); + } - for (call = __initcall_start; call < __initcall_end; call++) { - ktime_t t0, t1, delta; - char msgbuf[40]; - int result; + result = call(); - if (initcall_debug) { - print_fn_descriptor_symbol("calling %s()\n", - (unsigned long) *call); - t0 = ktime_get(); - } + if (initcall_debug) { + t1 = ktime_get(); + delta = ktime_sub(t1, t0); - result = (*call)(); + print_fn_descriptor_symbol("initcall %s()", + (unsigned long) call); + printk(" returned %d after %Ld msecs\n", result, + (unsigned long long) delta.tv64 >> 20); + } - if (initcall_debug) { - t1 = ktime_get(); - delta = ktime_sub(t1, t0); + msgbuf[0] = 0; - print_fn_descriptor_symbol("initcall %s()", - (unsigned long) *call); - printk(" returned %d after %Ld msecs\n", result, - (unsigned long long) delta.tv64 >> 20); - } + if (result && result != -ENODEV && initcall_debug) + sprintf(msgbuf, "error code %d ", result); - msgbuf[0] = 0; + if (preempt_count() != count) { + strlcat(msgbuf, "preemption imbalance ", sizeof(msgbuf)); + preempt_count() = count; + } + if (irqs_disabled()) { + strlcat(msgbuf, "disabled interrupts ", sizeof(msgbuf)); + local_irq_enable(); + } + if (msgbuf[0]) { + print_fn_descriptor_symbol(KERN_WARNING "initcall %s()", + (unsigned long) call); + printk(" returned with %s\n", msgbuf); + } +} - if (result && result != -ENODEV && initcall_debug) - sprintf(msgbuf, "error code %d ", result); +static void __init do_initcalls(void) +{ + initcall_t *call; - if (preempt_count() != count) { - strncat(msgbuf, "preemption imbalance ", sizeof(msgbuf)); - preempt_count() = count; - } - if (irqs_disabled()) { - strncat(msgbuf, "disabled interrupts ", sizeof(msgbuf)); - local_irq_enable(); - } - if (msgbuf[0]) { - print_fn_descriptor_symbol(KERN_WARNING "initcall %s()", - (unsigned long) *call); - printk(" returned with %s\n", msgbuf); - } - } + for (call = __initcall_start; call < __initcall_end; call++) + do_one_initcall(*call); /* Make sure there is no pending stuff from the initcall sequence */ flush_scheduled_work(); -- To unsubscribe from this list: send the line "unsubscribe linux-m68k" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html