Re: [PATCH] revision.c: Remove unneeded check for NULL

From: Jeff King <peff@peff.net>
Date: Jun 27, 2015, 06:07 UTC
Message-ID: <20150627060710.GA9353@peff.net>
In-Reply-To: <1435347619-29410-1-git-send-email-sbeller@google.com>

On Fri, Jun 26, 2015 at 12:40:19PM -0700, Stefan Beller wrote:

> 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.

Yeah, I think this is the right thing to do.

Acked-by: Jeff King <peff@peff.net>

Show 10 quoted lines

>     > 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.

Agreed. We do not assert non-NULL for every parameter in our functions, and I do not think this is any more special than the others.

>     > 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.

I kind of thought that the explanation in b6e8a3b covered this code. Does it not, or did people not read it?

The whole of revision.c is not well documented by comments, but I don't think that is a new thing.

-Peff