On Thu, Dec 3, 2015 at 9:06 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Stefan Naewe <stefan.naewe@xxxxxxxxx> 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@xxxxxxxxx> > > --- > > 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html