Re: [PATCH v2] revision.c: fix possible null pointer access
- From
Johannes Sixt <j6t@kdbg.org>
- Date
- Dec 7, 2015, 21:54 UTC
- Message-ID
- <5666000F.8050306@kdbg.org>
- In-Reply-To
- <xmqqegeym25s.fsf@gitster.mtv.corp.google.com>
Am 07.12.2015 um 21:31 schrieb Junio C Hamano:
Show 15 quoted lines
> Stefan Naewe <stefan.naewe@gmail.com> writes: > >> 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 <stefan.naewe@gmail.com> >> --- > > 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
Show 24 quoted lines
>
> Thanks.
>
>> 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;