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

Tom Tromey
2018-05-04T16:36:34Z

>>>>> "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