Re: [PATCH 5/5] gdb/python: extend the Python Disassembler API to allow for styling

Tom Tromey
2023-04-17T16:25:34Z

>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:

Andrew> This commit extends the Python Disassembler API to allow for styling
Andrew> of the instructions.

Thanks for doing this.

Andrew> + struct gdbarch *gdbarch = nullptr;
Andrew> + for (Py_ssize_t i = 0; i < parts_count; ++i)
Andrew> + {
Andrew> + gdbpy_ref<> part (PySequence_GetItem (parts_list, i));

PySequence_GetItem can fail, so this needs a check.

Andrew> +static PyObject *
Andrew> +disasmpy_addr_part_string (PyObject *self, void *closure)
Andrew> +{
Andrew> + disasm_addr_part_object *obj = (disasm_addr_part_object *) self;
Andrew> +
Andrew> + string_file buf;
Andrew> + print_address (obj->gdbarch, obj->address, &buf);

This should probably be in a try/catch. Unless the callee is known to
never throw, it's important to catch gdb exceptions when calling from
the Python layer into the core. Failure to do so will probably cause
crashes.

Andrew> +/* Describe the gdb.disassembler.DisassemblerPart type. */
Andrew> +
Andrew> +PyTypeObject disasm_part_object_type = {
Andrew> + PyVarObject_HEAD_INIT (nullptr, 0)
Andrew> + "gdb.disassembler.DisassemblerPart", /*tp_name*/
Andrew> + sizeof (PyObject), /*tp_basicsize*/
Andrew> + 0, /*tp_itemsize*/
Andrew> + 0, /*tp_dealloc*/
Andrew> + 0, /*tp_print*/
Andrew> + 0, /*tp_getattr*/
Andrew> + 0, /*tp_setattr*/
Andrew> + 0, /*tp_compare*/
Andrew> + 0, /*tp_repr*/

Should this one have a repr?
After the recent traffic about this I wonder if we should just mandate
that all new types have this slot set.

Tom