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

["Metzger, Markus T" <markus.t.metzger@intel.com>]

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.
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928