On Fri, Mar 27, 2015 at 03:32:48PM -0700, Stefan Beller wrote:
> `recent_bitmaps` is allocated in the function load_bitmap_entries_v1
> and it is not passed into any function, so it's safe to free it before
> leaving that function.
I think this is OK, though it might be easier still to just turn the
array into a stack variable. It's only 160 * sizeof(ptr), or about 1280
bytes on a 64-bit system. Note that the xcalloc there looks wrong (it is
way over-allocating by using the sizeof the struct, not a pointer to the
struct).
> Notes:
> I wonder however if we need to free the actual bitmaps
> stored in the recent_bitmaps as well.
No, those are just weak pointers. The memory is owned by the hash that
store_bitmap puts the bitmaps into.
> bitmap = read_bitmap_1(index);
This line allocates, too. So if we get down to...
> - if (xor_offset > MAX_XOR_OFFSET || xor_offset > i)
> - return error("Corrupted bitmap pack index");
> + if (xor_offset > MAX_XOR_OFFSET || xor_offset > i) {
> + ret = error("Corrupted bitmap pack index");
> + goto out;
> + }
...here, for example, where we have not yet called store_bitmap, then
it's a leak, too.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html