Hi,
Stefan Beller wrote:
> This was introduced at b6e8a3b5 (2015-04-17, limit_list: avoid
> quadratic behavior from still_interesting), which
> also introduced the check a few lines before, which already dereferences
> `interesting_cache`. So at this point `interesting_cache` is guaranteed to
> be not NULL.
The above is the rationale for the coverity warning, but it does not
explain why this change is safe.
> The code is called referencing the address of a local
> variable, so `interesting_cache` can actually never be NULL and trigger a
> segmentation fault by dereferencing it a few lines before this.
I'm having trouble parsing this sentence. Do you mean that limit_list()
only calls still_interesting() (and thus, indirectly,
everybody_uninteresting()), with the second parameter equal to the
address of the local interesting_cache variable, so it can never be
NULL?
That makes sense, but I had to look at the code and reread the above
sentence a few times before I understood.
Do you know what this code is trying to check for? What does it mean
for *interesting_cache to be NULL?
Should there be
if (!interesting_cache)
die("BUG: &interesting_cache == NULL");
checks at the top of still_interesting and everybody_uninteresting to
futureproof this?
What does the *interesting_cache variable represent, anyway?
This code seems to be underdocumented.
Thanks and hope that helps,
Jonathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
