[PATCH 4/5] Make Python inferior-related internal functions return a gdbpy_inf_ref
Simon Marchi
simon.marchi@polymtl.ca
Thu Feb 9 16:39:00 GMT 2017
Thanks for the comments. I'll update my branch, but I'll wait until
Tom's series is pushed and see what's still relevant in mine.
On 2017-02-09 07:30, Pedro Alves wrote:
>> @@ -207,39 +207,38 @@ python_new_objfile (struct objfile *objfile)
>> representing INFERIOR. If the object has already been created,
>> return it and increment the reference count, otherwise, create
>> it.
>> Return NULL on failure. */
>> -inferior_object *
>> +gdbpy_inf_ref
>> inferior_to_inferior_object (struct inferior *inferior)
>> {
> ...
>> - if (!inf_obj)
>> - return NULL;
>> + if (inf_obj == NULL)
>> + return gdbpy_inf_ref ();
>
> You shouldn't need changes like this one. gdbpy_ref has an
> implicit ctor that takes nullptr_t exactly to allow implicit
> construction from null.
Ok. This required adding the corresponding constructor in
gdbpy_ref_base:
gdbpy_ref_base (const std::nullptr_t)
: gdb::ref_ptr<T, gdbpy_ref_policy<T>> (nullptr)
{
}
>> /* Find thread entry in its inferior's thread_list. */
>> - for (entry = &inf_obj->threads; *entry != NULL; entry =
>> - &(*entry)->next)
>> + for (entry = &inf_obj_ref.get ()->threads;
>
> Hmm, changes like these are odd. gdbpy_ref has an operator->
> implementation, so inf_obj->threads should do the right thing?
Hmm you're right, not sure why I added those.
>> @@ -815,7 +809,10 @@ py_free_inferior (struct inferior *inf, void
>> *datum)
>> PyObject *
>> gdbpy_selected_inferior (PyObject *self, PyObject *args)
>> {
>> - return (PyObject *) inferior_to_inferior_object (current_inferior
>> ());
>> + gdbpy_inf_ref inf_obj_ref (inferior_to_inferior_object
>> (current_inferior ()));
>
> If the function returns gdbpy_inf_ref already, I much prefer
> using = initialization over (), like:
>
> gdbpy_inf_ref inf_obj_ref
> = inferior_to_inferior_object (current_inferior ());
>
> The reason is that this makes it more obvious what is going on.
> The ctor taking a PyObject* is explicit so inferior_to_inferior_object
> must be returning a gdbpy_inf_ref.
>
> With:
>
> gdbpy_inf_ref inf_obj_ref (inferior_to_inferior_object
> (current_inferior ()));
>
> one has to wonder what constructor is being called, and whether there's
> some kind of explicit conversion going on.
>
> So the = version is more to the point and thus makes it
> for a clearer read because there's less to reason about.
Right, it's more obvious.
Thanks,
Simon
More information about the Gdb-patches
mailing list