Re: New NULL dereference in sequence.c

From: Neil Horman <nhorman@tuxdriver.com>
Date: May 2, 2012, 19:38 UTC
Message-ID: <20120502193819.GB26732@hmsreliant.think-freely.org>
In-Reply-To: <vpqd36mcwjc.fsf@bauges.imag.fr>

On Wed, May 02, 2012 at 07:39:19PM +0200, Matthieu Moy wrote:

Show 17 quoted lines

> Neil Horman <nhorman@tuxdriver.com> writes:
> 
> >> >         if (!head_commit || parse_commit(head_commit))
> >> >             return error(_("could not parse commit %s\n"),
> >> >                      sha1_to_hex(head_commit->object.sha1));
> 
> [...]
> 
> > Have you actually seen this problem occur? It seems to me that the
> > only way head_commit could be NULL is in the event that HEAD wasn't a
> > commit object, whcih I don't think can be the case unless something
> > else is very wrong with your tree.
> 
> I don't know whether it can actually happen, but if it can't happen,
> then the if() condition should be rewritten. As it is, it explicitly
> allows head_commit to be NULL within the if body.
>

You're right, it should be reduced to just if (parse_commit(head_commit)) {} and we should call error with a different string dependent on weather head_commit is null or not.

I'll make a patch shortly. Neil

> -- 
> Matthieu Moy
> http://www-verimag.imag.fr/~moy/
>