Re: [PATCH][RESEND] mm: fix uninitialized variables for find_vma_prepare callers

["Ryan Hope" <rmh3093@gmail.com>]

Yeah original author should get credit, i just didnt go back an look,
thanks for doing that.

On Fri, Aug 1, 2008 at 1:31 PM, Hugh Dickins <hugh@veritas.com> wrote:
> 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;