public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Tom Tromey <tom@tromey.com>
To: Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org>
Cc: Andrew Burgess <aburgess@redhat.com>
Subject: Re: [PATCH 5/5] gdb/python: extend the Python Disassembler API to allow for styling
Date: Mon, 17 Apr 2023 10:25:12 -0600	[thread overview]
Message-ID: <87354y4gt3.fsf@tromey.com> (raw)
In-Reply-To: <62700c9aaddb83cc05e1b5e51e115d4eea8281e4.1680596378.git.aburgess@redhat.com> (Andrew Burgess via Gdb-patches's message of "Tue, 4 Apr 2023 09:21:07 +0100")

>>>>> "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

  parent reply	other threads:[~2023-04-17 16:25 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-04  8:21 [PATCH 0/5] Disassembler Styling And The Python API Andrew Burgess
2023-04-04  8:21 ` [PATCH 1/5] gdb/doc: improve Python Disassembler API documentation Andrew Burgess
2023-04-04 11:36   ` Eli Zaretskii
2023-04-28 22:49     ` Andrew Burgess
2023-05-12 17:42       ` Andrew Burgess
2023-04-04  8:21 ` [PATCH 2/5] gdb/python: implement __repr__ methods for py-disasm.c types Andrew Burgess
2023-05-12 17:43   ` Andrew Burgess
2023-04-04  8:21 ` [PATCH 3/5] gdb/python: implement DisassemblerResult.__str__ method Andrew Burgess
2023-05-12 17:43   ` Andrew Burgess
2023-04-04  8:21 ` [PATCH 4/5] gdb/python: rework how the disassembler API reads the result object Andrew Burgess
2023-04-04 11:38   ` Eli Zaretskii
2023-04-04  8:21 ` [PATCH 5/5] gdb/python: extend the Python Disassembler API to allow for styling Andrew Burgess
2023-04-04 12:01   ` Eli Zaretskii
2023-04-28 23:11     ` Andrew Burgess
2023-04-17 16:25   ` Tom Tromey [this message]
2023-04-28 23:09     ` [PATCHv2 " Andrew Burgess

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87354y4gt3.fsf@tromey.com \
    --to=tom@tromey.com \
    --cc=aburgess@redhat.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).