On Thu, Dec 3, 2015 at 9:06 PM, Junio C Hamano <gits...@pobox.com> wrote:
>
> Stefan Naewe <stefan.na...@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.na...@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.
OK.
>
> >
> > 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.
But 'tree->object' is dereferencing tree, isn't it ? Like '(*tree).object'.
??
> At least, please lose the blank line after
> the new assignment.
Will do, if you want this patch at all.
> > obj->flags |= UNINTERESTING;
>
> Thanks.
Thanks,
Stefan
--
----------------------------------------------------------------
python -c "print '73746566616e2e6e6165776540676d61696c2e636f6d'.decode('hex')"
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
