Re: [patch/rfc, rfa:doco, 6.0] "set backtrace past-main|limit"

Andrew Cagney
2003-08-04T13:17:24Z

>> Date: Sat, 02 Aug 2003 19:16:18 -0400
>> From: Andrew Cagney <ac131313@redhat.com>
>>
>> This patch replaces the commands:
>>
>> set/show backtrace-below-main
>> show backtrace-below-main
>>
>> with the pair of commands:
>>
>> set/show backtrace past-main
>> set/show backtrace limit
>
>
> I'm not sure I like what happens when the backtrace is deeper than
> the limit:
>
>
>> + if (this_frame->level > backtrace_limit)
>> + {
>> + error ("Backtrace limit of %d exceeded", backtrace_limit);
>> + }
>> +
>
>
> Why should this be anything as scary as `error'? Isn't a simple
> notice (not even a `warning') enough?

The choices I could think of were:

- warn and return NULL
but that would become tedious as it would keep occuring - get_prev_frame
is called many times.

- error out
perhaps add additional information on how to change the limit

- warn and continue the backtrace
I don't think this helps

The difference between a warning and error are largely internal - the
latter aborts the command and I think that's better here.

>> where the latter sets an absolute bound on the number of backtraces.
>> 10000 (arbitrary) by default.
>
>
> The 10000 default limit is not backward-compatible. Why not just
> leave it at zero, as that's how GDB behaves today?

(Zero gets turned into MAX_INT)

True, the problem I encountered was in the testsuite. That could always
be modified to, by default, set an upper bound.

> If we do set the limit by default to some number, that number should
> at least be documented in the manual.

There are a number of constants like that. There should be a more
robust way of keeping them consistent between the doco and the code.
However defaulting it to infinite would avoid the problem.

>> eli, the doco ok?
>
>
> Yes, but...
>
>
>> +If you need to examine the startup code, or limit the number of levels
>> +in a backtrace, you can change this behavior:
>>
>> @table @code
>> -@item set backtrace-below-main off
>> +@item set backtrace past-main off
>> Backtraces will stop when they encounter the user entry point. This is the
>> default.
>
>
> I think it's better to put "@itemx set backtrace past-main on" first,
> and "@item set backtrace past-main off" second, because otherwise the
> above fragment of text doesn't make sense: you tell the reader that
> the default behavior can be changed, but then show them the command
> that sets the default behavior.

I'll switch the order (which came from the old doco :-).

>> +@item set backtrace limit @var{number}
>> +@itemx set backtrace limit 0
>> +Limit the the backtrace to @var{number} levels. A value of zero means
>> +unlimited.
>
>
> I suggest a "@cindex backtrace limit" here. That way, someone who
> types "backtrace" in an Info reader and then hits TAB, will see this
> item in the list of possible completions.
>
> Also, isn't it better to use "n" instead of "number" here? It seems
> to me that
>
> Limit the backtrace to N levels
>
> sounds better in English than
>
> Limit the backtrace to NUMBER levels
>
> Do you agree?

Yep. I was wondering what @var{} to use there.

> Finally, note the two consecutive "the" before "backtrace"; a typo.
>
> Otherwise, this docs change is okay; thanks.

Thanks. Lets see if there are any other comments on limit's default.

Andrew