This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA] Expand "info record"
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...
--
Pedro Alves