{"thread":{"id":"40928","subject":"[PATCH] revision.c: fix possible null pointer access","startedAt":"2015-12-03T19:32:16Z","lastAt":"2015-12-07T21:54:23Z","messageCount":10,"participants":["Stefan Naewe","Junio C Hamano","Philip Oakley","Stefan Beller","Jeff King","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"273983","messageId":"1449171136-31566-1-git-send-email-stefan.naewe@gmail.com","threadId":"40928","inReplyTo":null,"subject":"[PATCH] revision.c: fix possible null pointer access","fromName":"Stefan Naewe","fromEmail":"stefan.naewe@gmail.com","sentAt":"2015-12-03T19:32:16Z","receivedAt":"2015-12-03T19:32:16Z","isPatch":true,"sender":{"key":"stefan.naewe@gmail.com","avatar":"https://avatars.githubusercontent.com/u/4468?v=4"},"body":"Two functions dereference a tree pointer before checking\nif the pointer is valid. Fix that by doing the check first.\n\nSigned-off-by: Stefan Naewe \n---\nThis has been reported through the CppHints newsletter (http://cpphints.com/hints/40)\nbut doesn't seem to have made its way to the ones who care (the git list\nthat is...)\n\n revision.c | 12 ++++++++++--\n 1 file changed, 10 insertions(+), 2 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex 0fbb684..bb40179 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -104,7 +104,12 @@ static void mark_tree_contents_uninteresting(struct tree *tree)\n {\n \tstruct tree_desc desc;\n \tstruct name_entry entry;\n-\tstruct object *obj = &tree->object;\n+\tstruct object *obj;\n+\n+\tif (!tree)\n+\t\treturn;\n+\n+\tobj = &tree->object;\n \n \tif (!has_sha1_file(obj->sha1))\n \t\treturn;\n@@ -135,10 +140,13 @@ static void mark_tree_contents_uninteresting(struct tree *tree)\n \n void mark_tree_uninteresting(struct tree *tree)\n {\n-\tstruct object *obj = &tree->object;\n+\tstruct object *obj;\n \n \tif (!tree)\n \t\treturn;\n+\n+\tobj = &tree->object;\n+\n \tif (obj->flags & UNINTERESTING)\n \t\treturn;\n \tobj->flags |= UNINTERESTING;\n-- \n2.6.3\n"},{"id":"273987","messageId":"xmqqlh9bthyb.fsf@gitster.mtv.corp.google.com","threadId":"40928","inReplyTo":"1449171136-31566-1-git-send-email-stefan.naewe@gmail.com","subject":"Re: [PATCH] revision.c: fix possible null pointer access","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-12-03T20:06:52Z","receivedAt":"2015-12-03T20:06:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Naewe writes:\n\n> Two functions dereference a tree pointer before checking\n\nReading them a bit carefully, a reader would notice that they\nactually do not dereference the pointer at all. It just computes\nanother pointer and that is done by adding the offset of object\nmember in the tree struct.\n\n> if the pointer is valid. Fix that by doing the check first.\n>\n> Signed-off-by: Stefan Naewe \n> ---\n> This has been reported through the CppHints newsletter (http://cpphints.com/hints/40)\n> but doesn't seem to have made its way to the ones who care (the git list\n> that is...)\n\nNobody would be surprised, unless the newsletter was sent to this\nlist, which I do not think it was (but if it was sent while I was\naway, then it is very possible that I didn't see it).\n\n> revision.c | 12 ++++++++++--\n> 1 file changed, 10 insertions(+), 2 deletions(-)\n>\n> diff --git a/revision.c b/revision.c\n> index 0fbb684..bb40179 100644\n> --- a/revision.c\n> +++ b/revision.c\n> @@ -104,7 +104,12 @@ static void mark_tree_contents_uninteresting(struct tree *tree)\n> {\n> \tstruct tree_desc desc;\n> \tstruct name_entry entry;\n> -\tstruct object *obj = &tree->object;\n> +\tstruct object *obj;\n> +\n> +\tif (!tree)\n> +\t\treturn;\n> +\n> +\tobj = &tree->object;\n\nThis is questionable; if you check all the callers of this function\n(there are two of them, I think), you would notice that they both\nknow that tree cannot be NULL here.\n\n> \n> \tif (!has_sha1_file(obj->sha1))\n> \t\treturn;\n> @@ -135,10 +140,13 @@ static void mark_tree_contents_uninteresting(struct tree *tree)\n> \n> void mark_tree_uninteresting(struct tree *tree)\n> {\n> -\tstruct object *obj = &tree->object;\n> +\tstruct object *obj;\n> \n> \tif (!tree)\n> \t\treturn;\n> +\n> +\tobj = &tree->object;\n> +\n> \tif (obj->flags & UNINTERESTING)\n> \t\treturn;\n\nThis one is not wrong per-se, but an unnecessary change, because no\ndeferencing is involved. At least, please lose the blank line after\nthe new assignment.\n\n> \tobj->flags |= UNINTERESTING;\n\nThanks.\n"},{"id":"273994","messageId":"CAJzBP5SNeuMcKLqdCBrHsYHdmOnuvyrvk7wao4b30Op2SE_ykw@mail.gmail.com","threadId":"40928","inReplyTo":"xmqqlh9bthyb.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] revision.c: fix possible null pointer access","fromName":"Stefan Naewe","fromEmail":"stefan.naewe@gmail.com","sentAt":"2015-12-03T21:15:56Z","receivedAt":"2015-12-03T21:15:56Z","isPatch":true,"sender":{"key":"stefan.naewe@gmail.com","avatar":"https://avatars.githubusercontent.com/u/4468?v=4"},"body":"On Thu, Dec 3, 2015 at 9:06 PM, Junio C Hamano wrote:\n>\n> Stefan Naewe writes:\n>\n> > Two functions dereference a tree pointer before checking\n>\n> Reading them a bit carefully, a reader would notice that they\n> actually do not dereference the pointer at all. It just computes\n> another pointer and that is done by adding the offset of object\n> member in the tree struct.\n>\n> > if the pointer is valid. Fix that by doing the check first.\n> >\n> > Signed-off-by: Stefan Naewe \n> > ---\n> > This has been reported through the CppHints newsletter (http://cpphints.com/hints/40)\n> > but doesn't seem to have made its way to the ones who care (the git list\n> > that is...)\n>\n> Nobody would be surprised, unless the newsletter was sent to this\n> list, which I do not think it was (but if it was sent while I was\n> away, then it is very possible that I didn't see it).\n>\n> > revision.c | 12 ++++++++++--\n> > 1 file changed, 10 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/revision.c b/revision.c\n> > index 0fbb684..bb40179 100644\n> > --- a/revision.c\n> > +++ b/revision.c\n> > @@ -104,7 +104,12 @@ static void mark_tree_contents_uninteresting(struct tree *tree)\n> > {\n> > struct tree_desc desc;\n> > struct name_entry entry;\n> > - struct object *obj = &tree->object;\n> > + struct object *obj;\n> > +\n> > + if (!tree)\n> > + return;\n> > +\n> > + obj = &tree->object;\n>\n> This is questionable; if you check all the callers of this function\n> (there are two of them, I think), you would notice that they both\n> know that tree cannot be NULL here.\n\nOK.\n\n>\n> >\n> > if (!has_sha1_file(obj->sha1))\n> > return;\n> > @@ -135,10 +140,13 @@ static void mark_tree_contents_uninteresting(struct tree *tree)\n> >\n> > void mark_tree_uninteresting(struct tree *tree)\n> > {\n> > - struct object *obj = &tree->object;\n> > + struct object *obj;\n> >\n> > if (!tree)\n> > return;\n> > +\n> > + obj = &tree->object;\n> > +\n> > if (obj->flags & UNINTERESTING)\n> > return;\n>\n> This one is not wrong per-se, but an unnecessary change, because no\n> deferencing is involved.\n\nBut 'tree->object' is dereferencing tree, isn't it ? Like '(*tree).object'.\n\n??\n\n> At least, please lose the blank line after\n> the new assignment.\n\nWill do, if you want this patch at all.\n\n> > obj->flags |= UNINTERESTING;\n>\n> Thanks.\n\nThanks,\n Stefan\n-- \n----------------------------------------------------------------\npython -c \"print '73746566616e2e6e6165776540676d61696c2e636f6d'.decode('hex')\"\n"},{"id":"273995","messageId":"46311B14CC814F54AC34764F2520947A@PhilipOakley","threadId":"40928","inReplyTo":"xmqqlh9bthyb.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] revision.c: fix possible null pointer access","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":null,"receivedAt":"2015-12-03T21:15:56Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Junio C Hamano\" \n> Stefan Naewe writes:\n>\n>> Two functions dereference a tree pointer before checking\n>\n> Reading them a bit carefully, a reader would notice that they\n> actually do not dereference the pointer at all. It just computes\n> another pointer and that is done by adding the offset of object\n> member in the tree struct.\n\nBut you can't do that computation (in the error case under consideration). \nNull can't be added to anything (as far as the implications of the standards \ngo). These are horrid gotchas because they go against the grain of all that \nbinary arithmetic and simplifications we learnt long ago.\n\nThat said, the fact that we know it can't be null does save the day, until \nthat is, the compiler [via some coding of an interpretation] decides that it \ncould be null and thus undefined etc etc (which one would argue as poor \nlogic, but standards have no truck with such arguments;-).\n\nThere were some discussion on undefined behaviour way back (2013-08-08) when \nStephan Beller looked at STACK's checking of the Git code, see for example \nhttp://article.gmane.org/gmane.comp.version-control.git/231945/\n\"3 issues have been discovered using the STACK tool\nThe paper regarding that tool can be found at\nhttps://pdos.csail.mit.edu/papers/stack:sosp13.pdf\" (link updated)\n\nAll their source code is publicly available at \nhttp://css.csail.mit.edu/stack/\n\n>\n>> if the pointer is valid. Fix that by doing the check first.\n>>\n>> Signed-off-by: Stefan Naewe \n>> ---\n>> This has been reported through the CppHints newsletter \n>> (http://cpphints.com/hints/40)\n>> but doesn't seem to have made its way to the ones who care (the git list\n>> that is...)\n>\n> Nobody would be surprised, unless the newsletter was sent to this\n> list, which I do not think it was (but if it was sent while I was\n> away, then it is very possible that I didn't see it).\n>\n>> revision.c | 12 ++++++++++--\n>> 1 file changed, 10 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/revision.c b/revision.c\n>> index 0fbb684..bb40179 100644\n>> --- a/revision.c\n>> +++ b/revision.c\n>> @@ -104,7 +104,12 @@ static void mark_tree_contents_uninteresting(struct \n>> tree *tree)\n>> {\n>> struct tree_desc desc;\n>> struct name_entry entry;\n>> - struct object *obj = &tree->object;\n>> + struct object *obj;\n>> +\n>> + if (!tree)\n>> + return;\n>> +\n>> + obj = &tree->object;\n>\n> This is questionable; if you check all the callers of this function\n> (there are two of them, I think), you would notice that they both\n> know that tree cannot be NULL here.\n>\n>>\n>> if (!has_sha1_file(obj->sha1))\n>> return;\n>> @@ -135,10 +140,13 @@ static void mark_tree_contents_uninteresting(struct \n>> tree *tree)\n>>\n>> void mark_tree_uninteresting(struct tree *tree)\n>> {\n>> - struct object *obj = &tree->object;\n>> + struct object *obj;\n>>\n>> if (!tree)\n>> return;\n>> +\n>> + obj = &tree->object;\n>> +\n>> if (obj->flags & UNINTERESTING)\n>> return;\n>\n> This one is not wrong per-se, but an unnecessary change, because no\n> deferencing is involved. At least, please lose the blank line after\n> the new assignment.\n>\n>> obj->flags |= UNINTERESTING;\n>\n> Thanks.\n\n--\nPhilip \n"},{"id":"273997","messageId":"CAGZ79kYRVDLooqTR2fRzoOVs2u2TeOubF9-wX9YVEVqHMOfT3Q@mail.gmail.com","threadId":"40928","inReplyTo":"46311B14CC814F54AC34764F2520947A@PhilipOakley","subject":"Re: [PATCH] revision.c: fix possible null pointer access","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-12-03T22:17:01Z","receivedAt":"2015-12-03T22:17:01Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Dec 3, 2015 at 1:34 PM, Philip Oakley wrote:\n> From: \"Junio C Hamano\" \n>>\n>> Stefan Naewe writes:\n>>\n>>> Two functions dereference a tree pointer before checking\n>>\n>>\n>> Reading them a bit carefully, a reader would notice that they\n>> actually do not dereference the pointer at all. It just computes\n>> another pointer and that is done by adding the offset of object\n>> member in the tree struct.\n\nWell compiler people want their compiler to produce the best output,\nmeaning the compiled code goes fast.\n\nSo if you ask a compiler-writer, this may qualify enough for\nbeing a dereference, because it looks like a dereference.\n\nAssuming this is a dereference, you can further reason about\nupcoming\n\n if (pointer)\n\nAs the pointer was already dereferenced, it can be assumed not NULL.\n(the pointer being NULL would be undefined behavior, in which the\ncompiler can do whatever it wants, i.e. that case can be ignored)\n\nSo with the strong assumption of the pointer being not NULL, you\ncan optimize away an\n\n if (pointer)\n\nas that is \"always\" false.\n\nIn case the pointer is NULL, we have had undefined behavior, so\nthe compiler is allowed to generate wrong code.\n\nWhich is why the if(pointer) is removed from the compiled binary,\nas less instructions make the code go faster.\n\n>\n> But you can't do that computation (in the error case under consideration).\n> Null can't be added to anything (as far as the implications of the standards\n> go). These are horrid gotchas because they go against the grain of all that\n> binary arithmetic and simplifications we learnt long ago.\n>\n> That said, the fact that we know it can't be null does save the day, until\n> that is, the compiler [via some coding of an interpretation] decides that it\n> could be null and thus undefined etc etc (which one would argue as poor\n> logic, but standards have no truck with such arguments;-).\n>\n> There were some discussion on undefined behaviour way back (2013-08-08) when\n> Stephan Beller looked at STACK's checking of the Git code, see for example\n> http://article.gmane.org/gmane.comp.version-control.git/231945/\n> \"3 issues have been discovered using the STACK tool\n> The paper regarding that tool can be found at\n> https://pdos.csail.mit.edu/papers/stack:sosp13.pdf\" (link updated)\n\n\nYeah that tool would detect such a bug. I can see\nif I can get it to run frequently and post results somewhere.\nIIRC it was quite a pain to get it working correctly on Git and\nthen reasoning for the resulting patch.\n\n>\n> All their source code is publicly available at\n> http://css.csail.mit.edu/stack/\n\nThanks for pointing to that tool again. :)\n"},{"id":"274005","messageId":"xmqq610ete8x.fsf@gitster.mtv.corp.google.com","threadId":"40928","inReplyTo":"46311B14CC814F54AC34764F2520947A@PhilipOakley","subject":"Re: [PATCH] revision.c: fix possible null pointer access","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-12-04T15:39:10Z","receivedAt":"2015-12-04T15:39:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Philip Oakley\" writes:\n\n> From: \"Junio C Hamano\" \n>> Stefan Naewe writes:\n>>\n>>> Two functions dereference a tree pointer before checking\n>>\n>> Reading them a bit carefully, a reader would notice that they\n>> actually do not dereference the pointer at all. It just computes\n>> another pointer and that is done by adding the offset of object\n>> member in the tree struct.\n>\n> But you can't do that computation (in the error case under\n> consideration). Null can't be added to anything (as far as the\n> implications of the standards go). These are horrid gotchas because\n> they go against the grain of all that binary arithmetic and\n> simplifications we learnt long ago.\n\nYeah, but in that hunk that does check !tree, because the function\ncan be fed a NULL, the computed result assigned to object, which is\nundefined, is never used ;-)\n\nOf course, there used to be exotic platforms that are still standard\ncompliant that triggered a trap when such a pointer computation was\nmade (rather, such a bogus pointer was assigned to a pointer\nvariable). I do not think anybody attempted to port Git to such a\nplatform, but I agree that it is better to \"fix\" such a codepath, if\nonly to stop wasting time dealing with them discussing with language\nlawyers ;-)\n\nSo as I said in my review, the first hunk is a reject, the second\none is OK.\n\nThanks.\n"},{"id":"274039","messageId":"20151204233255.GD15064@sigill.intra.peff.net","threadId":"40928","inReplyTo":"xmqq610ete8x.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] revision.c: fix possible null pointer access","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-12-04T23:32:55Z","receivedAt":"2015-12-04T23:32:55Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Dec 04, 2015 at 07:39:10AM -0800, Junio C Hamano wrote:\n\n> > But you can't do that computation (in the error case under\n> > consideration). Null can't be added to anything (as far as the\n> > implications of the standards go). These are horrid gotchas because\n> > they go against the grain of all that binary arithmetic and\n> > simplifications we learnt long ago.\n> \n> Yeah, but in that hunk that does check !tree, because the function\n> can be fed a NULL, the computed result assigned to object, which is\n> undefined, is never used ;-)\n> \n> Of course, there used to be exotic platforms that are still standard\n> compliant that triggered a trap when such a pointer computation was\n> made (rather, such a bogus pointer was assigned to a pointer\n> variable). I do not think anybody attempted to port Git to such a\n> platform, but I agree that it is better to \"fix\" such a codepath, if\n> only to stop wasting time dealing with them discussing with language\n> lawyers ;-)\n\nFWIW, I'd worry much more about compilers which do aggressive\noptimizations based on language-lawyering (e.g., removing the null-check\nas dead code, which is legal according to the standard because after you\ncomputed the pointer based on it, it's all undefined behavior).\n\nI don't think that changes your conclusion, though:\n\n> So as I said in my review, the first hunk is a reject, the second\n> one is OK.\n\n-Peff\n"},{"id":"274055","messageId":"1449329244-4585-1-git-send-email-stefan.naewe@gmail.com","threadId":"40928","inReplyTo":"xmqqlh9bthyb.fsf@gitster.mtv.corp.google.com","subject":"[PATCH v2] revision.c: fix possible null pointer access","fromName":"Stefan Naewe","fromEmail":"stefan.naewe@gmail.com","sentAt":"2015-12-05T15:27:24Z","receivedAt":"2015-12-05T15:27:24Z","isPatch":true,"sender":{"key":"stefan.naewe@gmail.com","avatar":"https://avatars.githubusercontent.com/u/4468?v=4"},"body":"mark_tree_uninteresting dereferences a tree pointer before checking\nif the pointer is valid. Fix that by doing the check first.\n\nSigned-off-by: Stefan Naewe \n---\n revision.c | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/revision.c b/revision.c\nindex 0fbb684..8c569cc 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -135,10 +135,12 @@ static void mark_tree_contents_uninteresting(struct tree *tree)\n \n void mark_tree_uninteresting(struct tree *tree)\n {\n-\tstruct object *obj = &tree->object;\n+\tstruct object *obj;\n \n \tif (!tree)\n \t\treturn;\n+\n+\tobj = &tree->object;\n \tif (obj->flags & UNINTERESTING)\n \t\treturn;\n \tobj->flags |= UNINTERESTING;\n-- \n2.6.3\n"},{"id":"274134","messageId":"xmqqegeym25s.fsf@gitster.mtv.corp.google.com","threadId":"40928","inReplyTo":"1449329244-4585-1-git-send-email-stefan.naewe@gmail.com","subject":"Re: [PATCH v2] revision.c: fix possible null pointer access","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-12-07T20:31:11Z","receivedAt":"2015-12-07T20:31:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Naewe writes:\n\n> mark_tree_uninteresting dereferences a tree pointer before checking\n> if the pointer is valid. Fix that by doing the check first.\n>\n> Signed-off-by: Stefan Naewe \n> ---\n\nI still have a problem with \"dereferences\", as \"dereference\" is\nabout computing an address and accessing memory based on the result,\nand only the first half is happening here. I can live with \"The\nfunction does a pointer arithmetic on 'tree' before it makes sure\nthat 'tree' is not NULL\", but in any case, let's queue this as-is\nfor now and wait for a while to see if others can come up with a\nmore appropriate phrases.\n\nThanks.\n\n> revision.c | 4 +++-\n> 1 file changed, 3 insertions(+), 1 deletion(-)\n>\n> diff --git a/revision.c b/revision.c\n> index 0fbb684..8c569cc 100644\n> --- a/revision.c\n> +++ b/revision.c\n> @@ -135,10 +135,12 @@ static void mark_tree_contents_uninteresting(struct tree *tree)\n> \n> void mark_tree_uninteresting(struct tree *tree)\n> {\n> -\tstruct object *obj = &tree->object;\n> +\tstruct object *obj;\n> \n> \tif (!tree)\n> \t\treturn;\n> +\n> +\tobj = &tree->object;\n> \tif (obj->flags & UNINTERESTING)\n> \t\treturn;\n> \tobj->flags |= UNINTERESTING;\n"},{"id":"274149","messageId":"5666000F.8050306@kdbg.org","threadId":"40928","inReplyTo":"xmqqegeym25s.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2] revision.c: fix possible null pointer access","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2015-12-07T21:54:23Z","receivedAt":"2015-12-07T21:54:23Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 07.12.2015 um 21:31 schrieb Junio C Hamano:\n> Stefan Naewe writes:\n>\n>> mark_tree_uninteresting dereferences a tree pointer before checking\n>> if the pointer is valid. Fix that by doing the check first.\n>>\n>> Signed-off-by: Stefan Naewe \n>> ---\n>\n> I still have a problem with \"dereferences\", as \"dereference\" is\n> about computing an address and accessing memory based on the result,\n> and only the first half is happening here. I can live with \"The\n> function does a pointer arithmetic on 'tree' before it makes sure\n> that 'tree' is not NULL\", but in any case, let's queue this as-is\n> for now and wait for a while to see if others can come up with a\n> more appropriate phrases.\n\nDon't shoo away language lawyers, because this is a pure C language rule \npatch. If this were only about pointer arithmetic, a change would not be \nnecessary. But it isn't. The patch corrects a case where the compiler \ncan remove a NULL pointer check that we actually want to remain. The \nlanguage rule that gives sufficient room for interpretation to the \ncompiler is about dereferencing a pointer. It is irrelevant that an \naddress of an object is taken after the dereference and then only \npointer arithmetic remains---the dereference has already taken place, \nand that cannot occur for a NULL pointer in a valid program. So, the \nphrase \"dereference\" is precise and correct here.\n\n-- Hannes\n\n>\n> Thanks.\n>\n>> revision.c | 4 +++-\n>> 1 file changed, 3 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/revision.c b/revision.c\n>> index 0fbb684..8c569cc 100644\n>> --- a/revision.c\n>> +++ b/revision.c\n>> @@ -135,10 +135,12 @@ static void mark_tree_contents_uninteresting(struct tree *tree)\n>>\n>> void mark_tree_uninteresting(struct tree *tree)\n>> {\n>> -\tstruct object *obj = &tree->object;\n>> +\tstruct object *obj;\n>>\n>> \tif (!tree)\n>> \t\treturn;\n>> +\n>> +\tobj = &tree->object;\n>> \tif (obj->flags & UNINTERESTING)\n>> \t\treturn;\n>> \tobj->flags |= UNINTERESTING;\n"}]}