On 05/21/2012 03:47 AM, Junio C Hamano wrote:
mhagger@xxxxxxxxxxxx writes:
I understand that it is not crucial to free memory allocated in a
cmd_*() function but it is unclear to me whether it is *preferred* to
let the process clean up take care of things.
Traditionally, the cmd_foo() functions roughly correspond to main() in
other programs, so from that point of view, "it is not crucial to free" is
an understatement. It is not even worth wasting people's time to (1)
decide which way is *preferred* and to (2) churn the code only to match
whichever way.
OK, thanks for the info. I will remove the "freeing-memory" part of the
patch.
If you have a plan to split cmd_fetch_pack() and make other parts of the
system call it, [...]
No, I have no plans for cmd_fetch_pack() besides cleaning up the
constness errors that I found when randomly reading the code.
It also seems that some cruft has snuck into this patch, e.g. like this
part,
- int i, ret, nr_heads;
+ int i, ret;
that do not have anything to do with "fix constness" nor "memory leak".
This particular hunk is part of moving alloc_heads, nr_heads, and heads
together to make it more obvious that they are part of an ALLOC_GROW
triplet. Previously alloc_heads was a block-local variable used only in
the case of the --stdin option.
But I admit that the patch is harder than necessary to read because of
the indentation changes etc, so I will break it up into more digestible
quanta.
Michael
--
Michael Haggerty
mhagger@xxxxxxxxxxxx
http://softwareswirl.blogspot.com/
--
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