From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 20980 invoked by alias); 29 Nov 2007 18:20:18 -0000 Received: (qmail 20969 invoked by uid 22791); 29 Nov 2007 18:20:17 -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; Thu, 29 Nov 2007 18:20:10 +0000 Received: (qmail 32615 invoked from network); 29 Nov 2007 18:20:08 -0000 Received: from unknown (HELO localhost) (jimb@127.0.0.2) by mail.codesourcery.com with ESMTPA; 29 Nov 2007 18:20:08 -0000 To: "Maciej W. Rozycki" Cc: gdb-patches@sourceware.org, "Maciej W. Rozycki" Subject: Re: Target-specific command logging facility References: From: Jim Blandy Date: Thu, 29 Nov 2007 18:20:00 -0000 In-Reply-To: (Maciej W. Rozycki's message of "Thu, 29 Nov 2007 15:44:23 +0000 (GMT)") 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-11/txt/msg00547.txt.bz2 "Maciej W. Rozycki" writes: > In preparation for submitting support for the MDI target I propose the > following enhancement to the command logging facility. Right now > serial_log_command() is called unconditionally which does nothing if the > serial target subsystem is not being used. > > I propose to change this behaviour and provide a (*to_log_command)() > member of the target vector so that the callee may be selected as > appropriate. The change below is meant to preserve the current semantics > -- all the currently supported targets that make use of the serial target > subsystem initialise the new member to serial_log_command(). > > Tested using the mipsisa32-sde-elf target, with the mips-sim-sde32/-EB > and mips-sim-sde32/-EL boards with no regressions. > > 2007-11-29 Maciej W. Rozycki > > * target.c (update_current_target): Inherit to_log_command. > * target.h (struct target_ops). Add to_log_command. > (target_log_command): New macro. > * top.c (execute_command): Call target_log_command() if available > rather than serial_log_command(). > * monitor.c (init_base_monitor_ops): Initialize to_log_command. > * remote-m32r-sdi.c (init_m32r_ops): Likewise. > * remote-mips.c (_initialize_remote_mips): Likewise. > * remote.c (init_remote_ops): Likewise. > > OK to apply? This looks good; I have some minor suggestions. > 14616.diff > Index: binutils-quilt/src/gdb/target.h > =================================================================== > --- binutils-quilt.orig/src/gdb/target.h 2007-04-16 13:04:50.000000000 +0100 > +++ binutils-quilt/src/gdb/target.h 2007-04-16 13:06:27.000000000 +0100 > @@ -402,6 +402,7 @@ > int); > struct exception_event_record *(*to_get_current_exception_event) (void); > char *(*to_pid_to_exec_file) (int pid); > + void (*to_log_command) (const char *); > enum strata to_stratum; > int to_has_all_memory; > int to_has_memory; > @@ -1201,6 +1202,10 @@ > > extern const struct target_desc *target_read_description (struct target_ops *); > > +/* Command logging facility. */ > + > +#define target_log_command current_target.to_log_command Could we have parens around the definition here? > Index: binutils-quilt/src/gdb/top.c > =================================================================== > --- binutils-quilt.orig/src/gdb/top.c 2007-04-16 12:14:49.000000000 +0100 > +++ binutils-quilt/src/gdb/top.c 2007-04-16 13:06:27.000000000 +0100 > @@ -387,7 +387,8 @@ > if (p == NULL) > return; > > - serial_log_command (p); > + if (target_log_command) > + target_log_command (p); I think it's better style to have the conditional in the definition of target_log_command itself, so that every caller doesn't have to test whether current target happens to have such a function. I grant this is the only caller, but people do look at existing code for precedent. If you fix those, this is fine to commit.