Re: [PATCH] Fix some sparse warnings

On 07/16/2013 10:53 PM, Philip Oakley wrote:

So I started using the clang code analyzer on git. One of the first warnings actually is this:

object.c:241:7: warning: Branch condition evaluates to a garbage value
                if (!eaten)

So that part of object.c lookx like this:
	struct object *parse_object(const unsigned char *sha1) 
	{
		int eaten;
		...
		obj = parse_object_buffer(sha1, type, size, buffer, &eaten);
		if (!eaten)
			free(buffer);
	}

And the parse_object_buffer looks like this with respect to the eaten 
variable:
	struct object *parse_object_buffer(...)
	{
		int eaten = 0;
		if (something)
			return NULL;
		...
		if (something_different)
			eaten=1;
		*eaten_p = eaten;
	}

So what might happen is, that parse_object_buffer exits early, without
executing 
	
	*eaten_p = eaten;

Then in the parse_object function eaten was never initialized nor set inside the call to parse_object_buffer. Then it is obvious that the free(buffer) is executed depending on garbage left on the stack. Definitely something what we want to change.

The obvious way to repair this would be to just initialize the eaten variable
inside parse_object.
	struct object *parse_object(const unsigned char *sha1) 
	{
		int eaten=0;
		...

However I'd like to propose another solution: In parse_object_buffer we do not have a local eaten variable, but directly write to *eaten_p. That would be the following patch.

Was there a particular idea or goal behind first having a local eaten variable, which later near the correct return of the function was used to set the eaten_p?

Thanks, Stefan