[PATCH] revision.c: Remove unneeded check for NULL
- From
Stefan Beller <sbeller@google.com>
- Date
- Jun 26, 2015, 19:40 UTC
- Message-ID
- <1435347619-29410-1-git-send-email-sbeller@google.com>
The function is called only from one place, which makes sure to have `interesting_cache` not NULL. Additionally it is a dereferenced a few lines before unconditionally, which would result in a segmentation fault.
Signed-off-by: Stefan Beller <sbeller@google.com> ---
Notes:
> So I think the right solution is just to drop the conditional entirely.
> The current code is not wrong (it is always a noop). What you have here
> actually misbehaves; it does not update the cache slot when it has
> become UNINTERESTING. That does not produce wrong results, but it loses
> the benefit of the cache in some cases.
After reading the code a bit more, I agree.
> 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?
I completely reworded the commit message.
> Should there be
>
> if (!interesting_cache)
> die("BUG: &interesting_cache == NULL");
>
> checks at the top of still_interesting and everybody_uninteresting to
> futureproof this?
I don't think this is necessary as these functions are local functions
so when somebody wants to use them they will be aware of the limitations.
> This code seems to be underdocumented.
I am not a expert in this area of the code, so I hoped Peff
would document it if he feels like so.revision.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/revision.c b/revision.c index 3ff8723..ab97ffd 100644 --- a/revision.c +++ b/revision.c @@ -361,8 +361,8 @@ static int everybody_uninteresting(struct commit_list *orig, list = list->next; if (commit->object.flags & UNINTERESTING) continue; - if (interesting_cache) - *interesting_cache = commit; + + *interesting_cache = commit; return 0; } return 1;
-- 2.4.1.345.gab207b6.dirty