Re: [PATCHv2 1/3] gdb/python: introduce gdb.Corefile API

[Andrew Burgess <aburgess@redhat.com>]

Tom Tromey <tom@tromey.com> writes:

>>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
>
> Andrew> This commit starts adding some core file related features to the
> Andrew> Python API.
>
> Thanks.
>
> Andrew> +/* Store a gdb.Corefile object in an inferior's registry.  */
> Andrew> +
> Andrew> +static const registry<inferior>::key<corefile_object,
> Andrew> +				     inferior_corefile_deleter>
> Andrew> +     cfpy_inferior_corefile_data_key;
>
> I suspect this should now use the new python-specific registry
> approaches, see gdbpy_registry.

I took a look at this, and I don't think gdbpy_registry is the right
solution.

  1. It is designed to hold objects which are stored on either an
     objfile or a gdbarch, e.g. types.  The use of templates allows us
     to avoid the duplication needed to deal with accessing the same
     type of "thing" either from the objfile or the gdbarch.  In my case
     I need to store on the inferior.

  2. It is designed for cases where each objfile or gdbarch can hold
     many "things" of a given type.  E.g. types, as such within each
     registry entry the "things" are stored in some container, like a
     map or set.  In my case each inferior has 0 or 1 associated core
     files, so adding a set or map would be unnecessary.

I'm sure with a little additional effort we _could_ generalise
gdbpy_registry more to make it apply in my case, but I don't think that
would actually add any value.  The existing code, for the cases in which
it applies, serves to remove some duplication.  But for cases like mine,
I don't think gdbpy_registry would remove any code.

The only benefit of extending gdbpy_registry would be that _every_ use
of a registry in Python code could, potentially, be made to use the same
interface.  But at a cost of making that interface more complex (to
handle the different cases).

Let me know your thoughts.

>
> Andrew> +  bfd *abfd= obj->inferior->pspace->core_bfd ();
>
> Nit: missing space before '='.

Fixed.

Thanks,
Andrew