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) for reporting errors and
>> various other purposes, e.g.
>> child = get_helper();
>> trace("started %s\n", child->argv);
>> if (finish_command(child))
>> return error("failed to cleanly finish %s", child->argv);
> 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
> 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