On 12/20/21 11:42 AM, Ævar Arnfjörð Bjarmason wrote:
On Mon, Dec 20 2021, Jeff Hostetler via GitGitGadget wrote:
From: Jeff Hostetler <jeffhost@xxxxxxxxxxxxx>
[...]
+
+void trace2_timer_stop(enum trace2_timer_id tid)
+{
+ if (!trace2_enabled)
+ return;
+
+ if (tid < 0 || tid >= TRACE2_NUMBER_OF_TIMERS)
+ BUG("invalid timer id: %d", tid);
nit / style: maybe assert() instead for cases where assert() produces
better info than BUG(). I.e. it would quote the whole expression, and
show you what condition it violated....
I'd rather leave it a BUG() so that we always get the
guard code. assert() goes away in non-debug builds and
a little while later "tid" will be used as a subscript.
I'll add the function name to the BUG message to make
it a little clearer.
[...]
Perhaps more readable/easily understood as just a (untested):
if (!t->interval_count || us_interval >= t->min_us)
t->min_us = us_interval;
if (!t->interval_count || us_interval >= t->max_us)
t->max_us = us_interval;
I.e. to avoid duplicating the identical assignment...
[...]
I'll look at something here to make this a little less
messy. Probably add a MIN() and MAX() to the mixture.
+ /*
+ * Number of nested starts on the stack in this thread. (We
+ * ignore recursive starts and use this to track the recursive
+ * calls.)
+ */
+ unsigned int recursion_count;
Earlier we have various forms of:
if (t->recursion_count > 1)
But since it's unsigned can we just make those a:
if (t->recursion_count)
The places that are > 0, yes. But the > 1 instances
are different since we're counting how many calls are
on the stack and want to handle recursive calls differently
than the first.
