From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27437 invoked by alias); 23 Nov 2014 07:44:57 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 27419 invoked by uid 89); 23 Nov 2014 07:44:56 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=AWL,BAYES_00 autolearn=ham version=3.3.2 X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Sun, 23 Nov 2014 07:44:55 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 661FB1167EC; Sun, 23 Nov 2014 02:44:53 -0500 (EST) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id ewFc-2CXEwGT; Sun, 23 Nov 2014 02:44:53 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id F30331167E3; Sun, 23 Nov 2014 02:44:52 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id 2096B40F79; Sun, 23 Nov 2014 11:44:52 +0400 (RET) Date: Sun, 23 Nov 2014 07:44:00 -0000 From: Joel Brobecker To: bug-hurd@gnu.org, thomas@codesourcery.com, gdb-patches@sourceware.org Subject: Re: [PATCH,Hurd] Fix deallocation after proc_getprocinfo call Message-ID: <20141123074452.GC7136@adacore.com> References: <20141102152537.GG2991@type.youpi.perso.aquilenet.fr> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20141102152537.GG2991@type.youpi.perso.aquilenet.fr> User-Agent: Mutt/1.5.21 (2010-09-15) X-SW-Source: 2014-11/txt/msg00556.txt.bz2 Hello Samuel, On Sun, Nov 02, 2014 at 04:25:37PM +0100, Samuel Thibault wrote: > 2014-10-02 Samuel Thibault > > * gdb/gnu-nat.c (inf_validate_procinfo): Multiply the number of > elements pi_len by the size of the elements before calling > vm_deallocate. > (inf_validate_task_sc): Likewise, and properly deallocate the > noise array. Again, sorry about the late review... I only have a few minor comments, almost trivial in nature. In the ChangeLog entry above, watch out that the last 2 lines are indented using spaces intead of tabs. > diff --git a/gdb/gnu-nat.c b/gdb/gnu-nat.c > index d17a750..c571190 100644 > --- a/gdb/gnu-nat.c > +++ b/gdb/gnu-nat.c > @@ -804,7 +804,7 @@ inf_validate_procinfo (struct inf *inf) > inf->nomsg = !!(pi->state & PI_NOMSG); > if (inf->nomsg) > inf->traced = !!(pi->state & PI_TRACED); > - vm_deallocate (mach_task_self (), (vm_address_t) pi, pi_len); > + vm_deallocate (mach_task_self (), (vm_address_t) pi, pi_len * sizeof (*(procinfo_t) 0)); The line is too long (soft limit is 74 characters, hard limit is 80). Suggest using "sizeof (struct procinfo)", which I think is better than dereferencing a NULL pointer. This is based on guessing that type procinfo_t is a pointer to struct procinfo, as suggested by the code in inf_validate_procinfo. > if (noise_len > 0) > vm_deallocate (mach_task_self (), (vm_address_t) noise, noise_len); > } > @@ -844,9 +844,9 @@ inf_validate_task_sc (struct inf *inf) > > suspend_count = pi->taskinfo.suspend_count; > > - vm_deallocate (mach_task_self (), (vm_address_t) pi, pi_len); > + vm_deallocate (mach_task_self (), (vm_address_t) pi, pi_len * sizeof (*(procinfo_t) 0)); Same as above. > if (noise_len > 0) > - vm_deallocate (mach_task_self (), (vm_address_t) pi, pi_len); > + vm_deallocate (mach_task_self (), (vm_address_t) noise, noise_len); > > if (inf->task->cur_sc < suspend_count) > { Thank you, -- Joel