On Tue, Nov 15, 2016 at 4:02 PM, Brandon Williams <bmwill@xxxxxxxxxx> wrote:
> On 11/15, Stefan Beller wrote:
>> +
>> + child_process_clear(&cp);
>> + return 0;
>> +}
>
> If run command is successful then it handles the clearing of the child
> process struct, correct? Is there a negative to having all the explicit
> clears when the child was successful?
void child_process_clear(struct child_process *child)
{
argv_array_clear(&child->args);
argv_array_clear(&child->env_array);
}
I don't think so, as clearing empty arg arrays is a no op.
>> +#define SCHEDULED_SUBMODULES_INIT {NULL, NULL}
>
> I may not know enough about these types of initializors but that Init
> macro only has 2 entries while there are three entries in the struct
> itself.
Filled up to 3 to be explicit.
>
>> +
>> +int scheduled_submodules_nr, scheduled_submodules_alloc;
>
> Should these globals be static since they should be scoped to only this
> file?
Of course, done.
>
> nit: organization wise it makes more sense to me to have the
> 'update_submodule' helper function be located more closely to the
> 'update_submodules' function.
>
done
David wrote:
> In fact, only the first NULL is necessary; unspecified initializer entries in C default to zero.
as said above, I explicitly init all of them now.