From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754072AbYGALxv (ORCPT ); Tue, 1 Jul 2008 07:53:51 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1753278AbYGALxm (ORCPT ); Tue, 1 Jul 2008 07:53:42 -0400 Received: from gw-colo-pa.panasas.com ([66.238.117.130]:24425 "EHLO natasha.panasas.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753176AbYGALxl (ORCPT ); Tue, 1 Jul 2008 07:53:41 -0400 Message-ID: <486A1AA3.203@panasas.com> Date: Tue, 01 Jul 2008 14:53:07 +0300 From: Benny Halevy User-Agent: Thunderbird 2.0.0.12 (X11/20080213) MIME-Version: 1.0 To: Hugh Dickins CC: Andrea Arcangeli , linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] mm: fix uninitialized variables for find_vma_prepare callers References: <1214844882-22560-1-git-send-email-bhalevy@panasas.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-OriginalArrivalTime: 01 Jul 2008 11:53:15.0991 (UTC) FILETIME=[0C0B3E70:01C8DB71] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Jun. 30, 2008, 22:00 +0300, Hugh Dickins wrote: > On Mon, 30 Jun 2008, Benny Halevy wrote: >> 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. > > That's amusing, thank you. > > You may wonder how the vma rb_tree has been working all these years > despite that. The answer is that we only use find_vma_prepare when > about to insert a new vma: if there's anything already there, it's > either an error condition, or we go off and unmap the overlap without > taking any interest in those uninitialized values for linking. > > It would be nicer to initialize them, and your patch is certainly > nice and simple. Would it have the effect, that it returns with > vma == *pprev when addr falls within an existing vma? > That would be a sensible outcome, I think. No, *pprev will be set to the last parent encountered with this patch, but doing what you suggested is possible, though I doubt it matters since apparently nobody is using pprev in this path otherwise stuff would've just broken. Benny > > Hugh > >> [warnings snipped] >> >> Signed-off-by: Benny Halevy >> --- >> mm/mmap.c | 2 +- >> 1 files changed, 1 insertions(+), 1 deletions(-) >> >> diff --git a/mm/mmap.c b/mm/mmap.c >> index 3354fdd..81b9873 100644 >> --- a/mm/mmap.c >> +++ b/mm/mmap.c >> @@ -366,7 +366,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;