On Wednesday 21 October 2009 02:31:41, Michael Snyder wrote:
Pedro Alves wrote:
On Tuesday 20 October 2009 23:25:30, Michael Snyder wrote:
+ /* Display instruction number for last instruction in the log. */
+ printf_filtered (_("Highest recorded instruction number is %llu.\n"),
+ record_insn_count ? record_insn_count - 1 : 0);
Why the conditional subtraction?
Because I don't want it to say "-1".
Okay, that much is obvious, but how can you reach here
with record_insn_count == 0, given that you check if you
have a log at all a bit above?
Maybe not -- but I'm a belt-and-suspenders guy.
I don't believe in not checking for something just because
it "can't happen". What if somebody changed the check above?
That's why we comment non-obvious code, or add
gdb_asserts (could be overkill), or warnings. The worse that can
happen if somebody changes the check above, is the new bug is
quickly exposed because gdb starting printing '(ULONGEST) -1'.
Masking a buglet like that is wrong, IMO.
Yeah, you've convinced me. Thanks for the prodding,
and please see new revision. I'll make that conditional
go away too. ;-)
Thanks. I could nit some more, but the all the info seems
there. ;-) I'm fine with the patch.
I don't see "info record" mentioned in the docs. Only:
@kindex info record insn-number
@item info record insn-number
Show the current number of recorded instructions.
Was there a corresponding documentation patch?
FTR, it still pains me when I see this:
+ if (current_target.to_stratum == record_stratum)
+ {
especially more so now that we have a stratum higher
than record_statum...