RE: [PATCH v10 05/10] python: Introduce gdb.RecordAuxiliary class.

Metzger, Markus T via Gdb-patches
2023-08-09T10:01:22Z

Hello Felix,

>> >> >@@ -455,10 +511,10 @@ btpy_list_item (PyObject *self, Py_ssize_t index)
>> >> >
>> >> > number = obj->first + (obj->step * index);
>> >> >
>> >> >- if (obj->element_type == &recpy_insn_type)
>> >> >- return recpy_insn_new (obj->thread, RECORD_METHOD_BTRACE,
>> number);
>> >> >- else
>> >> >+ if (obj->element_type == &recpy_func_type)
>> >> > return recpy_func_new (obj->thread, RECORD_METHOD_BTRACE,
>> number);
>> >> >+ else
>> >> >+ return btpy_item_new (obj->thread, number);
>> >> > }
>> >>
>> >> Why do we need to change the order?
>> >
>> >Because we no longer call recpy_insn_new only for recpy_insn_type but also for
>> >recpy_aux_type.
>>
>> I think it would be clearer if we checked each type separately and errored
>> out by default. Like a switch.
>
>I can put something like this:
>
> if (obj->element_type == &recpy_func_type)
> return recpy_func_new (obj->thread, RECORD_METHOD_BTRACE, number);
> else if (obj->element_type == &recpy_insn_type
> || obj->element_type == &recpy_aux_type)
> return btpy_item_new (obj->thread, number);
> else
> return PyErr_Format (gdbpy_gdb_error, _("Not a valid BtraceList object."));
>
>I don't think we can reach the error case right now though.

We will reach the error case when we add a new element type and forget to
update this function.

LGTM.

Maybe it makes sense to split btpy_item_new() again so we do not accidentally
create an insn object with a recpy_aux_type or vice versa.

How about aux objects in the function call history? This is supported on CLI if
I read patch 3 correctly.

Regards,
Markus.