Re: [PATCH] revision.c: fix possible null pointer access

>
> Stefan Naewe <stefan.naewe@gmail.com> writes:
>
> > Two functions dereference a tree pointer before checking
>
> Reading them a bit carefully, a reader would notice that they
> actually do not dereference the pointer at all.  It just computes
> another pointer and that is done by adding the offset of object
> member in the tree struct.
>
> > if the pointer is valid. Fix that by doing the check first.
> >
> > Signed-off-by: Stefan Naewe <stefan.naewe@gmail.com>
> > ---
> > This has been reported through the CppHints newsletter (http://cpphints.com/hints/40)
> > but doesn't seem to have made its way to the ones who care (the git list
> > that is...)
>
> Nobody would be surprised, unless the newsletter was sent to this
> list, which I do not think it was (but if it was sent while I was
> away, then it is very possible that I didn't see it).
>
> >  revision.c | 12 ++++++++++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/revision.c b/revision.c
> > index 0fbb684..bb40179 100644
> > --- a/revision.c
> > +++ b/revision.c
> > @@ -104,7 +104,12 @@ static void mark_tree_contents_uninteresting(struct tree *tree)
> >  {
> >       struct tree_desc desc;
> >       struct name_entry entry;
> > -     struct object *obj = &tree->object;
> > +     struct object *obj;
> > +
> > +     if (!tree)
> > +             return;
> > +
> > +     obj = &tree->object;
>
> This is questionable; if you check all the callers of this function
> (there are two of them, I think), you would notice that they both
> know that tree cannot be NULL here.

>
> >
> >       if (!has_sha1_file(obj->sha1))
> >               return;
> > @@ -135,10 +140,13 @@ static void mark_tree_contents_uninteresting(struct tree *tree)
> >
> >  void mark_tree_uninteresting(struct tree *tree)
> >  {
> > -     struct object *obj = &tree->object;
> > +     struct object *obj;
> >
> >       if (!tree)
> >               return;
> > +
> > +     obj = &tree->object;
> > +
> >       if (obj->flags & UNINTERESTING)
> >               return;
>
> This one is not wrong per-se, but an unnecessary change, because no
> deferencing is involved.