On Sat, Jun 8, 2013 at 6:32 AM, René Scharfe
> Am 08.06.2013 00:29, schrieb Felipe Contreras:
>> We are not freeing 'istate->cache' properly.
>> We can't rely on 'initialized' to keep track of the 'istate->cache',
>> because it doesn't really mean it's initialized. So assume it always has
>> data, and free it before overwriting it.
> That sounds a bit backwards to me. If ->initialized doesn't mean that the
> index state is initialized then something is fishy. Would it make sense and
> be sufficient to set ->initialized in add_index_entry?
I don't know.
> Or to get rid of it and check for ->cache_alloc instead?
That might make sense. I was thinking on renaming 'initialized' to
'loaded', but I really don't care.
>> Signed-off-by: Felipe Contreras <felipe.contre...@gmail.com>
>> read-cache.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>> diff --git a/read-cache.c b/read-cache.c
>> index 5e30746..a1dd04d 100644
>> --- a/read-cache.c
>> +++ b/read-cache.c
>> @@ -1451,6 +1451,7 @@ int read_index_from(struct index_state *istate,
>> const char *path)
>> istate->version = ntohl(hdr->hdr_version);
>> istate->cache_nr = ntohl(hdr->hdr_entries);
>> istate->cache_alloc = alloc_nr(istate->cache_nr);
>> + free(istate->cache);
>> istate->cache = xcalloc(istate->cache_alloc,
>> istate->initialized = 1;
> You wrote earlier that this change is safe with current callers and that it
> prevents leaks with the following sequence:
> # add entries
> Do we currently have such a call sequence somewhere?
I don't know.
> Wouldn't that be a
> bug, namely forgetting to call discard_cache before read_cache?
Why would it be a bug? There's nothing in the API that hints there's a
problem with that.
> I've added a "assert(istate->cache_nr == 0);" a few lines above and the test
> suite still passed. With the hunk below, ->cache is also always NULL and
> cache_alloc is always 0 at that point. So we don't need that free call
> there in the cases covered by the test suite at least -- better leave it
Why leave it out? If somebody makes the mistake of doing the above
sequence, would you prefer that we leak?
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html