Re: [PATCH 1/1] Fix Compilation Warning in kernel/trace/trace.c

[Steven Rostedt <rostedt@goodmis.org>]

On Fri, 2011-12-16 at 02:15 -0500, Devendra Naga wrote:
> there was a compilation warning saying
> kernel/trace/trace.c:3644:8: warning: ‘page2’ may be used uninitialized in this function

Nacked-by: Steven Rostedt <rostedt@goodmis.org>

And I also nacked this same fix here:

https://lkml.org/lkml/2011/11/28/287


> 
> The page2 pointer is used only when nr_pages = 2.
> 
> Assigned a null value to the page2 to get rid of warning.


Please please please, do not just fix warnings without actually looking
at the code.

1) this is a bug in the compiler, which happens to be fixed in newer
compilers (which does not produce this warning). It just happens that
gcc couldn't tell the simple usage of:

	if (nr_pages == 2)
		page2 = kmap_atomic(pages[1]);

and later:

	if (nr_pages == 2) {
		len = PAGE_SIZE - offset;
		memcpy(&entry->buf, page1 + offset, len);
		memcpy(&entry->buf[len], page2, cnt - len);

and finally:

	if (nr_pages == 2)
		kunmap_atomic(page2);

nr_pages never changes during this code. Gcc probably got confused
because on error exit, it does change (to unmap everything).

2) But your fix is still wrong!

Initializing it to NULL is wrong! Because it would hide a kernel bug if
page2 was really used uninitialized. As passing NULL as the source of
memcpy will panic the kernel.

Again, do not just randomly post fixes to gcc warnings without
understanding the real fix.

Thanks,

-- Steve



> 
> Signed-off-by: Devendra Naga <devendra.aaru@gmail.com>
> ---
>  kernel/trace/trace.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index f2bd275..80dfd92 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -3641,7 +3641,7 @@ tracing_mark_write(struct file *filp, const char __user *ubuf,
>  	int nr_pages = 1;
>  	ssize_t written;
>  	void *page1;
> -	void *page2;
> +	void *page2 = NULL;
>  	int offset;
>  	int size;
>  	int len;