This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA] clean up temp sals in handle_inferior_event


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]