From: Tom Tromey <tromey@redhat.com>
To: Michael Snyder <msnyder@vmware.com>
Cc: "gdb-patches\@sourceware.org" <gdb-patches@sourceware.org>
Subject: Re: [RFA] clean up temp sals in handle_inferior_event
Date: Tue, 21 Jul 2009 18:07:00 -0000 [thread overview]
Message-ID: <m3eisaf1zn.fsf@fleche.redhat.com> (raw)
In-Reply-To: <4A63D5B4.8090505@vmware.com> (Michael Snyder's message of "Sun\, 19 Jul 2009 19\:25\:56 -0700")
>>>>> "Michael" == Michael Snyder <msnyder@vmware.com> writes:
Michael> There were seven, and now eight, places in handle_inferior_event
Michael> where a local struct symtab_and_line is declared and used briefly.
Michael> One of them even opens and closes a block just for this purpose.
Michael> This patch merges those all into a single local temp variable.
Michael> They all initialize it, and all but one of them returns after
Michael> using it, so they can't interact. Plus I ran the testsuites
Michael> with no regressions.
I find the old code clearer. In the old code, these temporary variables
are mostly declared in small blocks surrounding their point of use.
This means that it is very easy to reason about the variable: its entire
lifetime fits onto the screen at once.
After the patch, this is not so. And, given that handle_inferior_event
is extremely long, I think this makes it trickier to understand.
I don't really do much work in this code, though, so perhaps I'm
speaking out of turn. I'd be interested to hear what Pedro has to say.
Also, if there is a benefit other than aesthetics to changing this, that
could change my opinion.
Tom
next prev parent reply other threads:[~2009-07-21 17:16 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-07-20 6:13 Michael Snyder
2009-07-21 18:07 ` Tom Tromey [this message]
2009-07-21 18:08 ` Stan Shebs
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=m3eisaf1zn.fsf@fleche.redhat.com \
--to=tromey@redhat.com \
--cc=gdb-patches@sourceware.org \
--cc=msnyder@vmware.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