On Tue, Nov 15, 2016 at 4:02 PM, Brandon Williams [off-list ref] wrote:

On 11/15, Stefan Beller wrote:

quoted

+
+     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.

quoted

+#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.

quoted

+
+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.