From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 80182 invoked by alias); 26 Jul 2016 11:14:29 -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 80169 invoked by uid 89); 26 Jul 2016 11:14:27 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=BAYES_00,KAM_STOCKGEN,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=no version=3.3.2 spammy=contract, face X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Tue, 26 Jul 2016 11:14:26 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 932F583F3A; Tue, 26 Jul 2016 11:14:25 +0000 (UTC) Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u6QBEOIO015219; Tue, 26 Jul 2016 07:14:24 -0400 Subject: Re: [RFA] PR python/18565 - make Frame.function work for inline frames To: Tom Tromey References: <1466439050-11330-1-git-send-email-tom@tromey.com> <86ziqfq6sz.fsf@gmail.com> <8737o5kqtv.fsf@tromey.com> <87bn1lu5h9.fsf@tromey.com> Cc: Yao Qi , "gdb-patches@sourceware.org" From: Pedro Alves Message-ID: <5ab877f8-9e39-6e02-2ece-46fcbcdacc0a@redhat.com> Date: Tue, 26 Jul 2016 11:14:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 MIME-Version: 1.0 In-Reply-To: <87bn1lu5h9.fsf@tromey.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2016-07/txt/msg00342.txt.bz2 On 07/25/2016 04:01 PM, Tom Tromey wrote: > *funname = xstrdup (SYMBOL_PRINT_NAME (func)); > [...] > if (*funlang == language_cplus) > { > /* It seems appropriate to use SYMBOL_PRINT_NAME() here, > to display the demangled name that we already have > stored in the symbol table, but we stored a version > with DMGL_PARAMS turned on, and here we don't want to > display parameters. So remove the parameters. */ > char *func_only = cp_remove_params (*funname); > > I'm not 100% sure that cp_remove_params cannot throw. > However, it's > simple to deal with this by adding a cleanup in find_frame_funname. I'm > happy to do this if desired. > > Another approach might be to have a free_current_contents cleanup at the > start of find_frame_funname and discard it at the exit. This would > maybe make it a bit safer in the face of future changes. Yet another approach would be to push the xstrdup call to after the cp_remove_params call, and remove the xfree call, something like: if (*funlang == language_cplus) { char *func_only = cp_remove_params (SYMBOL_PRINT_NAME (func)); if (func_only) *funname = func_only; else *funname = xstrdup (SYMBOL_PRINT_NAME (func)); } else *funname = xstrdup (SYMBOL_PRINT_NAME (func)); } In any case, IMO this would be the subject of a separate patch. > Alternatively, if we need a try/catch in the caller to possibly free the > function name, then several other callers are incorrect (ada-lang.c and > stack.c). Yeah. I think that if a function has such a requirement, then it needs to be clearly documented as that being part of its API contract. Otherwise, it's too easy for the called function to change in a way that makes the caller try to free a dangling output pointer. Thanks, Pedro Alves