From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25800 invoked by alias); 9 Oct 2008 01:20:58 -0000 Received: (qmail 25791 invoked by uid 22791); 9 Oct 2008 01:20:57 -0000 X-Spam-Check-By: sourceware.org Received: from smtp-outbound-2.vmware.com (HELO smtp-outbound-2.vmware.com) (65.115.85.73) by sourceware.org (qpsmtpd/0.31) with ESMTP; Thu, 09 Oct 2008 01:20:22 +0000 Received: from mailhost2.vmware.com (mailhost2.vmware.com [10.16.67.167]) by smtp-outbound-2.vmware.com (Postfix) with ESMTP id 35DBA59006; Wed, 8 Oct 2008 18:20:21 -0700 (PDT) Received: from [10.20.92.59] (promb-2s-dhcp59.eng.vmware.com [10.20.92.59]) by mailhost2.vmware.com (Postfix) with ESMTP id 2ADB28E562; Wed, 8 Oct 2008 18:20:21 -0700 (PDT) Message-ID: <48ED5BD0.7050107@vmware.com> Date: Thu, 09 Oct 2008 01:20:00 -0000 From: Michael Snyder User-Agent: Thunderbird 1.5.0.12 (X11/20080411) MIME-Version: 1.0 To: Pedro Alves CC: "gdb-patches@sourceware.org" Subject: Re: [RFA] Resubmit reverse debugging [4/5] References: <48EC18B9.5050209@vmware.com> <200810090135.56035.pedro@codesourcery.com> In-Reply-To: <200810090135.56035.pedro@codesourcery.com> Content-Type: text/plain; charset=UTF-8; format=flowed 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: 2008-10/txt/msg00267.txt.bz2 Pedro Alves wrote: >> +/* finish_backward -- helper function for finish_command. */ >> + >> +static void >> +finish_backward (struct symbol *function, struct thread_info *tp) >> +{ >> + struct symtab_and_line sal; >> + struct breakpoint *breakpoint; >> + struct cleanup *old_chain; >> + CORE_ADDR func_addr; >> + int back_up; >> + >> + if (find_pc_partial_function (get_frame_pc (get_current_frame ()), >> + NULL, &func_addr, NULL) == 0) >> + internal_error (__FILE__, __LINE__, >> + _("Finish: couldn't find function.")); >> + > > Still internal_error? Sorry, it's an artifact of the fact that I've been on a fork for so long. When I copied this code from finish_command, the code that I copied had a similar call to internal_error. In fact, finish_command_continuation still does. In fact, it's the same call that used to be in "finish_command". So what should it be? Just "error"? >> + sal = find_pc_line (func_addr, 0); >> + >> + /* TODO: Let's not worry about async until later. */ >> + > > Should be an error here instead of on finish_command ... > (keep reading) OK, I forgot to remove the comment... But I put the error in finish_command, because that is where all of the necessary information is available. In order to put the error here, I would have to add more function parameters and pass more information. I think I understand that you think it would be more "local" to put the error here -- but is it worth it if it makes us add complexity? finish_command already tests a number of things, including whether we are async and (now) whether we are reverse, and contains a number of error calls already.