From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9009 invoked by alias); 6 Aug 2007 22:28:35 -0000 Received: (qmail 8534 invoked by uid 22791); 6 Aug 2007 22:28:33 -0000 X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.31) with ESMTP; Mon, 06 Aug 2007 22:28:31 +0000 Received: (qmail 4883 invoked from network); 6 Aug 2007 22:28:29 -0000 Received: from unknown (HELO localhost) (jimb@127.0.0.2) by mail.codesourcery.com with ESMTPA; 6 Aug 2007 22:28:29 -0000 To: msnyder@sonic.net Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] solib_open, memory leak References: <21906.12.7.175.2.1186283848.squirrel@webmail.sonic.net> From: Jim Blandy Date: Mon, 06 Aug 2007 22:28:00 -0000 In-Reply-To: <21906.12.7.175.2.1186283848.squirrel@webmail.sonic.net> (msnyder@sonic.net's message of "Sat, 4 Aug 2007 20:17:28 -0700 (PDT)") Message-ID: User-Agent: Gnus/5.11 (Gnus v5.11) Emacs/22.0.50 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii 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/msg00125.txt.bz2 msnyder@sonic.net writes: > I hope it's not getting to be too late at night for me to do this stuff... > > If I'm not spacy, solib_open is leaking memory, because openp passes back > a malloc buffer for temp_pathname. In order to free it, it has to always > be a malloc buffer (hence no alloca etc). I think you were a little bit spacy. :) 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.