Messages in this thread Patch in this message | | | Date | Fri, 1 Aug 2008 18:31:37 +0100 (BST) | From | Hugh Dickins <> | Subject | Re: [PATCH][RESEND] mm: fix uninitialized variables for find_vma_prepare callers |
| |
On Fri, 1 Aug 2008, Ryan Hope wrote: > This was submitted for 2.6.26-rc8-mm1 but it must have gotten overlooked:
True, thanks, I guess that's my fault: it got stalled by me, because I found it unsatisfying to fix the uninitialization from a compiler point of view, whilst leaving it in an undefined state from a usability point of view. But since nothing does use those fields in this case, and my amateurish treephobic experiments didn't find a quick solution to that, I guess this is an improvement which should go in.
But please, let's credit Benny who posted it, and give his explanation of what the patch is for, with some additional comment from me. Which leaves you out - "Reminded-by: Ryan Hope <rmh3093@gmail.com>"?
From: Benny Halevy <bhalevy@panasas.com>
gcc 4.3.0 correctly emits the following warnings. When a vma covering addr is found, find_vma_prepare indeed returns without setting pprev, rb_link, and rb_parent.
/usr0/export/dev/bhalevy/git/linux-pnfs-bh-nfs41/mm/mmap.c: In function ‘insert_vm_struct’: /usr0/export/dev/bhalevy/git/linux-pnfs-bh-nfs41/mm/mmap.c:2085: warning: ‘rb_parent’ may be used uninitialized in this function /usr0/export/dev/bhalevy/git/linux-pnfs-bh-nfs41/mm/mmap.c:2085: warning: ‘rb_link’ may be used uninitialized in this function /usr0/export/dev/bhalevy/git/linux-pnfs-bh-nfs41/mm/mmap.c:2084: warning: ‘prev’ may be used uninitialized in this function /usr0/export/dev/bhalevy/git/linux-pnfs-bh-nfs41/mm/mmap.c: In function ‘copy_vma’: /usr0/export/dev/bhalevy/git/linux-pnfs-bh-nfs41/mm/mmap.c:2124: warning: ‘rb_parent’ may be used uninitialized in this function /usr0/export/dev/bhalevy/git/linux-pnfs-bh-nfs41/mm/mmap.c:2124: warning: ‘rb_link’ may be used uninitialized in this function /usr0/export/dev/bhalevy/git/linux-pnfs-bh-nfs41/mm/mmap.c:2123: warning: ‘prev’ may be used uninitialized in this function /usr0/export/dev/bhalevy/git/linux-pnfs-bh-nfs41/mm/mmap.c: In function ‘do_brk’: /usr0/export/dev/bhalevy/git/linux-pnfs-bh-nfs41/mm/mmap.c:1951: warning: ‘rb_parent’ may be used uninitialized in this function /usr0/export/dev/bhalevy/git/linux-pnfs-bh-nfs41/mm/mmap.c:1951: warning: ‘rb_link’ may be used uninitialized in this function /usr0/export/dev/bhalevy/git/linux-pnfs-bh-nfs41/mm/mmap.c:1949: warning: ‘prev’ may be used uninitialized in this function /usr0/export/dev/bhalevy/git/linux-pnfs-bh-nfs41/mm/mmap.c: In function ‘mmap_region’: /usr0/export/dev/bhalevy/git/linux-pnfs-bh-nfs41/mm/mmap.c:1092: warning: ‘rb_parent’ may be used uninitialized in this function /usr0/export/dev/bhalevy/git/linux-pnfs-bh-nfs41/mm/mmap.c:1092: warning: ‘rb_link’ may be used uninitialized in this function /usr0/export/dev/bhalevy/git/linux-pnfs-bh-nfs41/mm/mmap.c:1089: warning: ‘prev’ may be used uninitialized in this function
Hugh adds: in fact, none of find_vma_prepare's callers use those values when a vma is found to be already covering addr, it's either an error or an occasion to munmap and repeat. Okay, let's quieten the compiler (but I would prefer it if pprev, rb_link and rb_parent were meaningful in that case, rather than whatever's in them from descending the tree).
Signed-off-by: Benny Halevy <bhalevy@panasas.com> Signed-off-by: Hugh Dickins <hugh@veritas.com> ---
mm/mmap.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
--- 2.6.27-rc1/mm/mmap.c +++ linux/mm/mmap.c @@ -370,7 +370,7 @@ find_vma_prepare(struct mm_struct *mm, unsigned long addr, if (vma_tmp->vm_end > addr) { vma = vma_tmp; if (vma_tmp->vm_start <= addr) - return vma; + break; __rb_link = &__rb_parent->rb_left; } else { rb_prev = __rb_parent; | |