From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11095 invoked by alias); 10 Mar 2009 18:00:19 -0000 Received: (qmail 11045 invoked by uid 22791); 10 Mar 2009 18:00:17 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL,BAYES_00,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 10 Mar 2009 18:00:07 +0000 Received: (qmail 5412 invoked from network); 10 Mar 2009 18:00:04 -0000 Received: from unknown (HELO orlando.local) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 10 Mar 2009 18:00:04 -0000 From: Pedro Alves To: teawater Subject: Re: [RFA] Submit process record and replay third time, 3/9 Date: Tue, 10 Mar 2009 19:35:00 -0000 User-Agent: KMail/1.9.10 Cc: gdb-patches@sourceware.org, Michael Snyder References: <200903092034.57367.pedro@codesourcery.com> In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Content-Disposition: inline Message-Id: <200903101800.08633.pedro@codesourcery.com> 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: 2009-03/txt/msg00145.txt.bz2 On Tuesday 10 March 2009 17:02:46, teawater wrote: > >> +#define TARGET_IS_PROCESS_RECORD =A0 \ > >> + =A0 =A0 (current_target.beneath =3D=3D &record_ops) > > > > Sorry, but I repeat the request I've made several times already. =A0Thi= s is > > not the right way to do this. =A0You need to add a new target_ops metho= d or > > property that the core of GDB checks on. =A0It is not correct that make > > the core of GDB reference record_ops directly. =A0To come up with > > the target callback's name, at each call site of TARGET_IS_PROCESS_RECO= RD, > > consider what is the property of the current target that GDB needs to > > know about the current target. =A0Is it something like: > > > > =A0target_is_recording () ? > > =A0target_is_replaying () ? > > =A0target_is_read_only () ? > > > > etc. > > >=20 > I forget a process record has special strata "record_stratum". >=20 > What about delete "TARGET_IS_PROCESS_RECORD" and add > #define target_is_record (target) (target->to_stratum =3D=3D record_strat= um) > to target.h? No, that's not a new callback... If we in some hypothetical future end up layering yet another target on top of record, then that check will fail. What I'm saying is, TARGET_IS_PROCESS_RECORD is just as wrong as TARGET_IS_LINUX_NAT_C, or TARGET_IS_WINDOWS_NAT_C. E.g., in places in the common code that we want to check if the current target has execution, we call target_has_execution, doesn't matter which target it is, as long as it has execution. If we want to check that the target is in asynchronous more, we check for target_is_async_p, again, doesn't matter which target it is. In your case, imagine that you implemented all of record.c in gdbserver instead of on GDB. Say, imagine that there's no record.c in GDB at all. Then, when you connected to gdbserver, and told it to start recording, the topmost pushed target on the GDB side would still be the remote target (process stratum). GDB wouldn't know how gdbserver was implementing the recording feature, only that the remote side supports the recording feature. Now, see, here's a property we'd possibly want to expose through a target method --- e.g., target_can_record_p(). If you look at the places in the core of GDB where you are currently checking for TARGET_IS_PROCESS_RECORD, and you wanted those checks to also return true in the case of precord being implemented on the remote server, clearly, you'd need to check for some target property other than the target name, or its stratum. Why is it that you need to call TARGET_IS_PROCESS_RECORD in the first place? That is the key here. Again, is it because the target is recording, and you can't do some things in that case? Is it because the target is replaying? Or perhaps it's a more fundamental state --- is the target in a read only state? When you find out which *state(s)* you're interesting in checking, we can easily add target method(s) for it/them, and make the record target implement them (say, returning true), and making the default return false. Or, we may even come to the conclusion that is not a target method we want --- we may end up with a global variable, similar to `execution_direction'. --=20 Pedro Alves