From: Jeff King <peff@peff.net>
To: Stefan Beller <sbeller@google.com>
Cc: gitster@pobox.com, git@vger.kernel.org, jrnieder@gmail.com
Subject: Re: [PATCH] revision.c: Remove unneeded check for NULL
Date: Sat, 27 Jun 2015 02:07:10 -0400 [thread overview]
Message-ID: <20150627060710.GA9353@peff.net> (raw)
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>
> > 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
next prev parent reply other threads:[~2015-06-27 6:07 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-26 19:40 [PATCH] revision.c: Remove unneeded check for NULL Stefan Beller
2015-06-27 6:07 ` Jeff King [this message]
2015-06-30 22:48 ` Jonathan Nieder
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150627060710.GA9353@peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jrnieder@gmail.com \
--cc=sbeller@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).