Re: [PATCH v4 3/9] add target method delegation

[Pedro Alves <palves at redhat dot com>]

On 10/28/2013 05:51 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> Could you please split out the patch for the second problem?  I
> Pedro> think it'll be a small one.
> 
> Sure.
> 
> Pedro> What's the plan for the existing target methods that
> Pedro> currently already do a similar beneath lookup?  I'd like it that
> Pedro> there's be at least a plan, so we don't end up with yet another
> Pedro> way of doing things, two incomplete transitions, and no clear direction.
> 
> Do you mean things like target_detach?

Yes.

> 
> If so, then I think these are two different things.
> 
> target.h declares things both for users of the target API and for the
> implementation of targets.
> 
> Something like target_detach is a public API.  My understanding of the
> current scheme is that a public-facing method can either be implemented
> by a function like target_detach, which encodes all the needed logic; or
> by a macro that just calls into the target_ops, in which case the method
> must use the inherit/default machinery.

Right.  I'm thinking the public functions that do the run
time target beneath walk, instead of the inherit/default
mechanism, and about having them defer to the delegation version,
avoiding the need to implement basically the same twice.

Like, to take a simple example:

int
target_has_all_memory_1 (void)
{
  struct target_ops *t;

  for (t = current_target.beneath; t != NULL; t = t->beneath)
    if (t->to_has_all_memory (t))
      return 1;

  return 0;
}

becomes:

#define target_has_all_memory_1 \
  target_delegate_has_all_memory (&current_target)

target_detach becomes (completely untested):

void
target_detach (char *args, int from_tty)
{
  struct target_ops* t;

  if (gdbarch_has_global_breakpoints (target_gdbarch ()))
    /* Don't remove global breakpoints here.  They're removed on
       disconnection from the target.  */
    ;
  else
    /* If we're in breakpoints-always-inserted mode, have to remove
       them before detaching.  */
    remove_breakpoints_pid (ptid_get_pid (inferior_ptid));

  prepare_for_detach ();

  target_delegate_detach (&current_target, args, from_tty);

  if (targetdebug)
    fprintf_unfiltered (gdb_stdlog, "target_detach (%s, %d)\n",
			args, from_tty);
}

etc.  So basically, whether that was in the plan.  Is
there something about the delegate mechanism that would
make this not work?  If so, could we make it work?  Having
this considered was the sort of plan thing I had in mind.

-- 
Pedro Alves