Florian Achleitner <florian.achleitner.2.6...@gmail.com> writes:

>> The updated code frees argv[] immediately after start_command()
>> returns, and it may happen to be safe to do so with the current
>> implementation of start_command() and friends, but I think it is a
>> bad taste to free argv[] (or env[] for that matter) before calling
>> finish_command().  These pieces of memory are still pointed by the
>> child_process structure, and users of the structure may want to use
>> contents of them (especially, argv[0]) for reporting errors and
>> various other purposes, e.g.
>>      child = get_helper();
>>         trace("started %s\n", child->argv[0]);
>>      if (finish_command(child))
>>              return error("failed to cleanly finish %s", child->argv[0]);
> Yes, sounds reasonable. The present of immedate clearing has the advantage 
> that I don't have to store the struct argv_array, as struct child_process 
> only 
> has a member for const char **argv.

And updated code shouldn't have to store struct argv_array either.
If you just give the ownership of argv_array.argv to child_process
and clean it as part of destroying the child_process, you do not
have to worry about argv_array at all.

In order to cleanly support that use case at the API level, we may
want to introduce argv_array_detach() that is similar in spirit to
strbuf_detach(), which transfers ownership of the underlying memory
to the caller.
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

Reply via email to