From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6425 invoked by alias); 16 Apr 2008 18:56:01 -0000 Received: (qmail 6417 invoked by uid 22791); 16 Apr 2008 18:56:00 -0000 X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.31) with ESMTP; Wed, 16 Apr 2008 18:55:43 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id C83462A9667; Wed, 16 Apr 2008 14:55:37 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id LiFtRNNva+ko; Wed, 16 Apr 2008 14:55:37 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 8743E2A9660; Wed, 16 Apr 2008 14:55:37 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 66CB5E7ACD; Wed, 16 Apr 2008 11:55:35 -0700 (PDT) Date: Wed, 16 Apr 2008 21:16:00 -0000 From: Joel Brobecker To: Doug Evans Cc: gdb-patches@sourceware.org Subject: Re: [RFA] mixed source+assembly from cli disassemble Message-ID: <20080416185535.GB3626@adacore.com> References: <20080404003857.A5A451C72B9@localhost> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20080404003857.A5A451C72B9@localhost> User-Agent: Mutt/1.4.2.2i 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/msg00307.txt.bz2 Doug, > 2008-04-03 Doug Evans > > * cli/cli-cmds.c (print_disassembly): New fn. > (disassemble_current_function): New fn. > (disassemble_command): Recognize /s modifier, print mixed > source+assembly. > (init_cli_cmds): Update disassemble help text. MichaelS already approved the patch, and I agree with him. I had some comments, and a request so I'm sending them now. First, the request: Can you also write up a NEWS entry? I think this is a great addition and it deserves a mention in NEWS. The ChangeLog is missing the entry for the documentation update. Please make sure that EliZ has seen it before checking in. I'm not sure whether this is the case or not anymore. I also personally prefer to avoid abbreviations such as "fn", although the GNU coding standards say nothing against them - so it may only be a personally preference. I see that it has been use a few times in the past, but most uses is in the 90s. Don't change it unless other maintainers agree that it should be changed. > + printf_filtered ("from "); > + deprecated_print_address_numeric (low, 1, gdb_stdout); > + printf_filtered (" to "); > + deprecated_print_address_numeric (high, 1, gdb_stdout); > + printf_filtered (":\n"); While you are modifying this code, I think the following should work too: printf_filtered ("form %s to %s:\n", paddress (low), paddress (high)); Not only does this help us get rid of a couple of uses of deprecated functions, I believe it's also better for i18n. Other than that, thanks for doing the work! I'll definitely enjoy this new feature :). -- Joel