lkml.org 
[lkml]   [2011]   [Dec]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [PATCH 1/1] Fix Compilation Warning in kernel/trace/trace.c
From
Date
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;


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2011-12-16 13:43    [W:0.030 / U:5.092 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site