From: Jeff King Date: Wed, 12 Nov 2014 10:52:29 GMT Subject: Re: [PATCH v2 1/2] Clean stale environment pointer in finish_command() Message-ID: <20141112105229.GA29101@peff.net> In-Reply-To: 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