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

Am 07.12.2015 um 21:31 schrieb Junio C Hamano:

Stefan Naewe [off-list ref] writes:

quoted

mark_tree_uninteresting dereferences a tree pointer before checking
if the pointer is valid. Fix that by doing the check first.

Signed-off-by: Stefan Naewe <redacted>
---

I still have a problem with "dereferences", as "dereference" is
about computing an address and accessing memory based on the result,
and only the first half is happening here.  I can live with "The
function does a pointer arithmetic on 'tree' before it makes sure
that 'tree' is not NULL", but in any case, let's queue this as-is
for now and wait for a while to see if others can come up with a
more appropriate phrases.

Don't shoo away language lawyers, because this is a pure C language rule 
patch. If this were only about pointer arithmetic, a change would not be 
necessary. But it isn't. The patch corrects a case where the compiler 
can remove a NULL pointer check that we actually want to remain. The 
language rule that gives sufficient room for interpretation to the 
compiler is about dereferencing a pointer. It is irrelevant that an 
address of an object is taken after the dereference and then only 
pointer arithmetic remains---the dereference has already taken place, 
and that cannot occur for a NULL pointer in a valid program. So, the 
phrase "dereference" is precise and correct here.

-- Hannes

Thanks.

quoted

  revision.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/revision.c b/revision.c
index 0fbb684..8c569cc 100644
--- a/revision.c
+++ b/revision.c

@@ -135,10 +135,12 @@ 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;
  	obj->flags |= UNINTERESTING;