[PATCH] Fix uninitialized variable warning in do_one_initcall

[PATCH] Fix uninitialized variable warning in do_one_initcall

[Kevin Winchester]

The warning is corrected by extracting the debug path out into
its own function.  This does en up duplicating one line of code
between the debug and regular code paths (i.e. the actual call
of the initcall function), but it seems worthwhile for the
cleaner build.

Signed-off-by: Kevin Winchester <kjwinchester@gmail.com>
---
 init/main.c |   49 ++++++++++++++++++++++++++++---------------------
 1 files changed, 28 insertions(+), 21 deletions(-)

diff --git a/init/main.c b/init/main.c
index a42fdf4..21def42 100644
--- a/init/main.c
+++ b/init/main.c
@@ -728,30 +728,38 @@ static char msgbuf[64];
 static struct boot_trace_call call;
 static struct boot_trace_ret ret;
 
+static inline int do_one_initcall_debug(initcall_t fn)
+{
+	ktime_t calltime, rettime, delta;
+	int result;
+
+	call.caller = task_pid_nr(current);
+	printk(KERN_DEBUG "calling  %pF @ %i\n", fn, call.caller);
+	calltime = ktime_get();
+	trace_boot_call(&call, fn);
+	enable_boot_trace();
+
+	result = fn();
+
+	disable_boot_trace();
+	rettime = ktime_get();
+	delta = ktime_sub(rettime, calltime);
+	ret.duration = (unsigned long long) ktime_to_ns(delta) >> 10;
+	trace_boot_ret(&ret, fn);
+	printk(KERN_DEBUG "initcall %pF returned %d after %lld usecs\n", fn,
+		ret.result, ret.duration);
+
+	return result;
+}
+
 int do_one_initcall(initcall_t fn)
 {
 	int count = preempt_count();
-	ktime_t calltime, delta, rettime;
-
-	if (initcall_debug) {
-		call.caller = task_pid_nr(current);
-		printk("calling  %pF @ %i\n", fn, call.caller);
-		calltime = ktime_get();
-		trace_boot_call(&call, fn);
-		enable_boot_trace();
-	}
 
-	ret.result = fn();
-
-	if (initcall_debug) {
-		disable_boot_trace();
-		rettime = ktime_get();
-		delta = ktime_sub(rettime, calltime);
-		ret.duration = (unsigned long long) ktime_to_ns(delta) >> 10;
-		trace_boot_ret(&ret, fn);
-		printk("initcall %pF returned %d after %Ld usecs\n", fn,
-			ret.result, ret.duration);
-	}
+	if (initcall_debug)
+		ret.result = do_one_initcall_debug(fn);
+	else
+		ret.result = fn();
 
 	msgbuf[0] = 0;
 
@@ -773,7 +781,6 @@ int do_one_initcall(initcall_t fn)
 	return ret.result;
 }
 
-
 extern initcall_t __initcall_start[], __initcall_end[], __early_initcall_end[];
 
 static void __init do_initcalls(void)
-- 
1.7.1

Re: [PATCH] Fix uninitialized variable warning in do_one_initcall

[Andrew Morton]

On Sat,  3 Jul 2010 20:26:28 -0300
Kevin Winchester <kjwinchester@gmail.com> wrote:

> The warning is corrected by extracting the debug path out into
> its own function.  This does en up duplicating one line of code
> between the debug and regular code paths (i.e. the actual call
> of the initcall function), but it seems worthwhile for the
> cleaner build.
> 

I assume the warning was for `calltime'?  Maybe other things?  Please,
remove all doubt and always quote the compiler output in the changelog.

Also please mention the compiler version - it looks like this is a new
warning.  It's not a false positive either - the compiler doesn't know
that initcall_debug's value never changes.

The patch doesn't apply to linux-next because someone went on a great
tromp through do_one_initcall() so could you please redo the patch
against linux-next?

I suggest that you not inline do_one_initcall_debug() - the compiler
will do that anwyay.

And if you're feeling keen, please do a separate patch which marks
do_one_initcall() and do_one_initcall_debug() with __init_or_module -
we don't need to leave that code in memory after bootup if
CONFIG_MODULES=n.

Thanks.

[PATCH 1/2] Fix warning: 'calltime.tv64' may be used uninitialized in this function in init/main.c

[Kevin Winchester]

Using:

	gcc (GCC) 4.5.0 20100610 (prerelease)

The following warning appears:

	init/main.c: In function ‘do_one_initcall’:
	init/main.c:730:10: warning: ‘calltime.tv64’ may be used uninitialized in this function

This warning is actually correct, as the global initcall_debug
could arguably be changed by the initcall.

Correct this warning by extracting a new function,
do_one_initcall_debug, that performs the initcall for the debug
case.

Signed-off-by: Kevin Winchester <kjwinchester@gmail.com>
---
 init/main.c |   34 ++++++++++++++++++++--------------
 1 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/init/main.c b/init/main.c
index 3e0f4b5..828ffac 100644
--- a/init/main.c
+++ b/init/main.c
@@ -724,27 +724,33 @@ core_param(initcall_debug, initcall_debug, bool, 0644);
 
 static char msgbuf[64];
 
-int do_one_initcall(initcall_t fn)
+static int do_one_initcall_debug(initcall_t fn)
 {
-	int count = preempt_count();
 	ktime_t calltime, delta, rettime;
 	unsigned long long duration;
 	int ret;
 
-	if (initcall_debug) {
-		printk("calling  %pF @ %i\n", fn, task_pid_nr(current));
-		calltime = ktime_get();
-	}
-
+	printk(KERN_DEBUG "calling  %pF @ %i\n", fn, task_pid_nr(current));
+	calltime = ktime_get();
 	ret = fn();
+	rettime = ktime_get();
+	delta = ktime_sub(rettime, calltime);
+	duration = (unsigned long long) ktime_to_ns(delta) >> 10;
+	printk(KERN_DEBUG "initcall %pF returned %d after %lld usecs\n", fn,
+		ret, duration);
 
-	if (initcall_debug) {
-		rettime = ktime_get();
-		delta = ktime_sub(rettime, calltime);
-		duration = (unsigned long long) ktime_to_ns(delta) >> 10;
-		printk("initcall %pF returned %d after %lld usecs\n", fn,
-			ret, duration);
-	}
+	return ret;
+}
+
+int do_one_initcall(initcall_t fn)
+{
+	int count = preempt_count();
+	int ret;
+
+	if (initcall_debug)
+		ret = do_one_initcall_debug(fn);
+	else
+		ret = fn();
 
 	msgbuf[0] = 0;
 
-- 
1.7.1

[PATCH 2/2] Mark do_one_initcall* as __init_or_module

[Kevin Winchester]

Andrew Morton suggested that the do_one_initcall and
do_one_initcall_debug functions can be marked __init_or_module
such that they can be discarded for the CONFIG_MODULES=N case.

Signed-off-by: Kevin Winchester <kjwinchester@gmail.com>
---
 init/main.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/init/main.c b/init/main.c
index 828ffac..f466d04 100644
--- a/init/main.c
+++ b/init/main.c
@@ -724,7 +724,7 @@ core_param(initcall_debug, initcall_debug, bool, 0644);
 
 static char msgbuf[64];
 
-static int do_one_initcall_debug(initcall_t fn)
+static int __init_or_module do_one_initcall_debug(initcall_t fn)
 {
 	ktime_t calltime, delta, rettime;
 	unsigned long long duration;
@@ -742,7 +742,7 @@ static int do_one_initcall_debug(initcall_t fn)
 	return ret;
 }
 
-int do_one_initcall(initcall_t fn)
+int __init_or_module do_one_initcall(initcall_t fn)
 {
 	int count = preempt_count();
 	int ret;
-- 
1.7.1