From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6533 invoked by alias); 7 Apr 2008 19:43:23 -0000 Received: (qmail 6520 invoked by uid 22791); 7 Apr 2008 19:43:22 -0000 X-Spam-Check-By: sourceware.org Received: from smtp-out.google.com (HELO smtp-out.google.com) (216.239.33.17) by sourceware.org (qpsmtpd/0.31) with ESMTP; Mon, 07 Apr 2008 19:43:05 +0000 Received: from zps18.corp.google.com (zps18.corp.google.com [172.25.146.18]) by smtp-out.google.com with ESMTP id m37Jgv1E006402 for ; Mon, 7 Apr 2008 20:42:58 +0100 Received: from wa-out-1112.google.com (wahv33.prod.google.com [10.114.248.33]) by zps18.corp.google.com with ESMTP id m37JfpIY006299 for ; Mon, 7 Apr 2008 12:42:57 -0700 Received: by wa-out-1112.google.com with SMTP id v33so1265103wah.2 for ; Mon, 07 Apr 2008 12:42:57 -0700 (PDT) Received: by 10.114.197.1 with SMTP id u1mr346004waf.156.1207597375841; Mon, 07 Apr 2008 12:42:55 -0700 (PDT) Received: by 10.115.107.18 with HTTP; Mon, 7 Apr 2008 12:42:55 -0700 (PDT) Message-ID: Date: Mon, 07 Apr 2008 22:56:00 -0000 From: "Doug Evans" To: gdb-patches@sourceware.org Subject: Re: [RFA] source.c:find_and_open_source cleanup In-Reply-To: <20080407193412.3CDC11C751F@localhost> MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <20080407193412.3CDC11C751F@localhost> 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: 2008-04/txt/msg00140.txt.bz2 Presumably this is just leftovers from the mstrsave removal. The code was originally something like: if (result >= 0) { char *tmp_fullname; tmp_fullname = *fullname; *fullname = mstrsave (objfile->md, *fullname); xfree (tmp_fullname); } On Mon, Apr 7, 2008 at 12:34 PM, Doug Evans wrote: > This is odd: > > if (result >= 0) > { > char *tmp_fullname; > tmp_fullname = *fullname; > *fullname = xstrdup (tmp_fullname); > xfree (tmp_fullname); > } > > Is there something subtle going on here? > I thought maybe the caller might be expecting a newly allocated value > if it passed in a value for `fullname', but the previous value has > already been freed at this point. > > 2008-04-07 Doug Evans > > * source.c (find_and_open_source): Add some comments clarifying > handling of FULLNAME argument. Make static. Remove pointless > xstrdup/xfree. > > Index: source.c > =================================================================== > RCS file: /cvs/src/src/gdb/source.c,v > retrieving revision 1.86 > diff -u -p -u -p -r1.86 source.c > --- source.c 14 Mar 2008 18:39:43 -0000 1.86 > +++ source.c 7 Apr 2008 19:30:01 -0000 > @@ -927,15 +927,19 @@ rewrite_source_path (const char *path) > DIRNAME is the compilation directory of a particular source file. > Only some debug formats provide this info. > FULLNAME can be the last known absolute path to the file in question. > + Space for the path must have been malloc'd. If a path substitution > + is applied we free the old value and set a new one. > > On Success > A valid file descriptor is returned. ( the return value is positive ) > FULLNAME is set to the absolute path to the file just opened. > + The caller is responsible for freeing FULLNAME. > > On Failure > An invalid file descriptor is returned. ( the return value is negative ) > FULLNAME is set to NULL. */ > -int > + > +static int > find_and_open_source (struct objfile *objfile, > const char *filename, > const char *dirname, > @@ -1022,13 +1026,6 @@ find_and_open_source (struct objfile *ob > result = openp (path, OPF_SEARCH_IN_PATH, p, OPEN_MODE, 0, fullname); > } > > - if (result >= 0) > - { > - char *tmp_fullname; > - tmp_fullname = *fullname; > - *fullname = xstrdup (tmp_fullname); > - xfree (tmp_fullname); > - } > return result; > } > >