On 07/16/2013 10:53 PM, Philip Oakley wrote:
> 
> Does anyone run the "new static checker called 'Stack' that precisely
> identifies unstable code"? [though the paper's conclusion says 'All
> Stack source code will be publicly available.' which suggests it's not
> yet available]
> 

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