Re: [PATCH v2 1/2] Clean stale environment pointer in finish_command()

[Jeff King <peff@peff.net>]

On Wed, Nov 12, 2014 at 11:45:19AM +0100, Johannes Schindelin wrote:

> Okay, I have to say that I was led to believe that reusing the
> child_process struct is okay because argv_array_clear() explicitly
> reinitializes the env_array field, something that is useless churn unless
> you plan to reuse the memory.

The argv_array code prepares its data structure for reuse after freeing.
But child_process which uses it does not make any such promises. If
there were an argv_array_free(), child_process could use it. But there
is only argv_array_clear().

I don't have a problem with finish_command leaving its child_process in
a known usable state (e.g., by calling child_process_init). But I also
don't see much benefit.

> However, my personal taste says that reusing the same memory is more
> elegant than to waste extra memory unnecessarily, so I will go with the
> child_process_init() solution.

I do not mind much either way. But I doubt that a single extra struct on
the stack will break the bank, compared to the fact that we are forking
and execing a new program. I'd also not be surprised if a smart compiler
could notice that the variables are used exclusively in non-overlapping
bits of the code, and just reuse the stack space.

-Peff