Re: [PATCH 3/3] Heap-allocate core_target instances

[Tom Tromey <tom@tromey.com>]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> This gets rid of the core_ops global, and replaces it with
Pedro> heap-allocated core_target instances.  In practice, there will only be
Pedro> one such instance, though that will change further ahead as more
Pedro> pieces of multi-target support are merged.

Pedro> +  /* FIXME: kettenis/20031023: Eventually this variable should
Pedro> +     disappear.  */
Pedro> +  struct gdbarch *m_core_gdbarch = NULL;

This comment is very gdb.


It was a bit hard to read the patch but I think I excerpted this correctly:

Pedro> +void
Pedro> +core_target::close ()
[...]
Pedro> +  delete this;

I realize this is what was planned from your c++-ification series, but
today I was wondering if this code path:

    void
    core_target::detach (inferior *inf, int from_tty)
    {
      unpush_target (this);

  =>

    int
    unpush_target (struct target_ops *t)
    ...
      target_close (t);

invokes undefined behavior.

"delete this" seems to be ok if you are careful not to use "this"
afterward, but is this particular use ok?  I don't know the rule here
but I thought I'd mention it just in case.

Pedro> +/* Deleter for std::unique_ptr.  Closes the target if an exception is
Pedro> +   thrown.  */
Pedro> +struct target_ops_closer
Pedro> +{
Pedro> +  void operator() (target_ops *target)
Pedro> +  {
Pedro> +    target->close ();
Pedro> +  }
Pedro> +};

This seems like something to put in target.h.

Tom