This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFC - Python Scripting] New method gdb.Architecture.disassemble
>>>>> "Siva" == Siva Chandra <sivachandra@google.com> writes:
Siva> Thanks for your detailed reply. I now have a patch which does not use
Siva> ui_out. Instead of calling gdb_disassembly, it essentially
Siva> re-implements disasm.c:dump_insns. The patch is attached.
I don't have any problem with this approach.
I do have some nits on the patch though.
Siva> +/* Implementation of gdb.Architecture.disassemble (self, low, high) -> List.
Siva> + Returns a list of instructions in a memory address range. Each instruction
Siva> + in the list is a dict object with the following string keys:
Siva> +
You can write a shorter comment here and just put all the informative
stuff into the docs.
Siva> + static char unknown_str[] = { '<', 'u', 'n', 'k', 'n', 'o', 'w', 'n', '>', '\0' };
This line looks too long.
I think it isn't too hard to reorganize so that this can be an ordinary
const char *.
Siva> + result_list = PyList_New (0);
Siva> + if (!result_list)
Siva> + {
Siva> + PyErr_SetString (PyExc_MemoryError,
Siva> + _("Unable to create a list of disassembled "
Siva> + "instructions."));
Siva> + return NULL;
You don't need PyErr_SetString here.
PyList_New will have done that already.
So just return NULL.
Siva> + struct ui_file *memfile = mem_fileopen ();
Siva> + PyObject *insn_dict = PyDict_New ();
Siva> + volatile struct gdb_exception except;
Siva> +
Siva> + if (!insn_dict)
Siva> + {
Siva> + Py_DECREF (result_list);
Siva> + PyErr_SetString (PyExc_MemoryError,
Siva> + _("Unable to create a dict for instruction."));
Siva> +
Siva> + return NULL;
Here too.
This leaks memfile on failure.
Siva> + }
Siva> + if (PyList_Append (result_list, insn_dict))
Siva> + {
Siva> + Py_DECREF (result_list);
Siva> + Py_DECREF (insn_dict);
Siva> +
Siva> + return NULL; /* PyList_Append Sets the exception. */
This leaks it too.
Siva> + if (funcname)
Siva> + xfree (funcname);
Siva> + if (filename)
Siva> + xfree (filename);
Siva> + if (asm_code)
Siva> + xfree (asm_code);
xfree handles NULL arguments fine, so remove the "if"s.
Siva> + if (PyDict_SetItemString (insn_dict, "addr",
Siva> + gdb_py_long_from_ulongest (pc))
Siva> + || PyDict_SetItemString (insn_dict, "asm",
Siva> + PyString_FromString (asm_code))
I think PyString_FromString handles const char * arguments fine.
So you can make this:
asm_code == NULL ? unknown_str : asm_code
and avoid some hair above and below, and fix up unknown_str as well.
Siva> + {
Siva> + Py_DECREF (result_list);
Siva> + PyErr_SetString (PyExc_MemoryError,
Siva> + _("Unable to add fields to instruction dict."));
Siva> +
Siva> + return NULL;
You don't need the PyErr_SetString.
And this leaks some memory.
Siva> + if (funcname && funcname != unknown_str)
Siva> + xfree (funcname);
Siva> + if (filename && filename != unknown_str)
Siva> + xfree (filename);
Siva> + if (asm_code && asm_code != unknown_str)
Siva> + xfree (asm_code);
No need to check nullity; but with the other changes this could all be
unconditional.
Tom