public inbox for archer@sourceware.org
 help / color / mirror / Atom feed
From: Tom Tromey <tromey@redhat.com>
To: Phil Muldoon <pmuldoon@redhat.com>
Cc: archer@sourceware.org, oguzkayral@gmail.com
Subject: Re: Python inferior control
Date: Fri, 13 Aug 2010 19:49:00 -0000	[thread overview]
Message-ID: <m3ocd6xpgp.fsf@fleche.redhat.com> (raw)
In-Reply-To: <4C655CA0.8010205@redhat.com> (Phil Muldoon's message of "Fri, 13 Aug 2010 15:54:24 +0100")

>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:

Phil> I've not changed very much: renamed the files, re-ordered the
Phil> Makefile.in and made some minor code changes so that it compiles.

Thanks for doing this.

I skimmed through it.  Some of the formatting is weird, maybe due to
wrapping in the original posts?  Anyway, be sure to fix all that up.

It still seems like a lot of code to define a new event.  But I think we
will just live with that.  Or maybe there is some way to reduce its
size, through macros or something.  There seems to be a fair amount of
boilerplate.

There are places where the error-checking is not correct.  E.g.,
emit_breakpoint_stop_event calls PyObject_CallObject but does not check
its return value.  (FWIW I think that errors in events should be
reported and then ignored -- not propagate.)

I see new attributes like "stop_eventregistry".  I think we should drop
the "registry" part of these names.

I see that all the existing event registries are on the Thread object.
I think this is fine, but I think we will also want to add more.  In
particular:

* We want some way to determine that the entire inferior has exited.
* It seems logical to want to be able to put an event handler on a
  breakpoint, to notice when that breakpoint is hit.

There are probably more of these.  I don't mind if we add them as needed
-- but it would be nice if the first release incorporating the event
machinery were complete enough to enable some "nice" scripts.

I am not sure about all of the logic in emit_stop_event.  Is this a full
enumeration of stop reasons?  Also I am unsure about python_thread_exit.
Looking at $_exitcode seems strange -- a thread doesn't really have an
exit code, only the inferior as a whole does.

One place to look for ideas here is how MI does it...


I'm afraid there will probably be more things once the above are fixed
up.  Due to the formatting and lack of docs, this patch was sort of
difficult to read.  But, I think the basics are ok.

Tom

  reply	other threads:[~2010-08-13 19:49 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-08-13 14:54 Phil Muldoon
2010-08-13 19:49 ` Tom Tromey [this message]
2010-08-26 15:02   ` Oguz Kayral
2010-08-26 20:28     ` Tom Tromey
2010-09-20 16:13       ` Matt Rice
2010-09-21 21:48         ` Tom Tromey
2010-08-15 17:42 ` Richard Ward
2010-08-26 20:23   ` Tom Tromey
2010-08-25 16:10 ` Phil Muldoon
2010-12-01 18:58 ` sami wagiaalla

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=m3ocd6xpgp.fsf@fleche.redhat.com \
    --to=tromey@redhat.com \
    --cc=archer@sourceware.org \
    --cc=oguzkayral@gmail.com \
    --cc=pmuldoon@redhat.com \
    /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).