Messages in this thread Patch in this message | | | Subject | Re: [PATCH 1/1] Fix Compilation Warning in kernel/trace/trace.c | From | Steven Rostedt <> | Date | Fri, 16 Dec 2011 07:40:59 -0500 |
| |
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/
| |