From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 716 invoked by alias); 6 Aug 2007 22:39:11 -0000 Received: (qmail 588 invoked by uid 22791); 6 Aug 2007 22:39:10 -0000 X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (66.187.233.31) by sourceware.org (qpsmtpd/0.31) with ESMTP; Mon, 06 Aug 2007 22:39:05 +0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.13.1/8.13.1) with ESMTP id l76Md2ca010334 for ; Mon, 6 Aug 2007 18:39:02 -0400 Received: from pobox.corp.redhat.com (pobox.corp.redhat.com [10.11.255.20]) by int-mx1.corp.redhat.com (8.13.1/8.13.1) with ESMTP id l76Md2Xu019129 for ; Mon, 6 Aug 2007 18:39:02 -0400 Received: from ironwood.lan (vpn-14-45.rdu.redhat.com [10.11.14.45]) by pobox.corp.redhat.com (8.13.1/8.13.1) with ESMTP id l76Md1ZS023998 for ; Mon, 6 Aug 2007 18:39:01 -0400 Date: Mon, 06 Aug 2007 22:39:00 -0000 From: Kevin Buettner To: gdb-patches@sources.redhat.com Subject: Re: [PATCH] solib_open, memory leak Message-ID: <20070806153901.37fe366b@ironwood.lan> In-Reply-To: References: <21906.12.7.175.2.1186283848.squirrel@webmail.sonic.net> X-Mailer: Sylpheed-Claws 2.6.0 (GTK+ 2.10.4; i386-redhat-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-IsSubscribed: yes 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 X-SW-Source: 2007-08/txt/msg00129.txt.bz2 On Mon, 06 Aug 2007 15:28:28 -0700 Jim Blandy wrote: > Even if you've made sure that temp_pathname is malloc'd by the time we > reach the 'open', any later 'openp' call will throw away its value. > openp is careful to store *something* in the pointer referred to by > its last argument, even on error. > > I think the invariant should be that, when found_file becomes >= 0, > then temp_pathname is malloc'd, and not before. The 'openp' clauses > will preserve that. So I think you need: > > if (found_file >= 0) > temp_pathname = xstrdup (temp_pathname); > > after the 'open'. > > And then there's no need for the xstrdup at the bottom; just return > temp_pathname, or xfree it if the caller doesn't want it. Jim's analysis looks right to me. Kevin