On Wed, Sep 21, 2016 at 11:20 PM, Jeff King <peff@xxxxxxxx> wrote: >> +/** >> + * Recursively call ls-files on a submodule >> + */ >> +static void show_gitlink(const struct cache_entry *ce) >> +{ >> + struct child_process cp = CHILD_PROCESS_INIT; >> + int status; >> + >> + argv_array_push(&cp.args, "ls-files"); >> + argv_array_push(&cp.args, "--recurse-submodules"); >> + argv_array_pushf(&cp.args, "--submodule-prefix=%s%s/", >> + submodule_prefix ? submodule_prefix : "", >> + ce->name); >> + cp.git_cmd = 1; >> + cp.dir = ce->name; >> + status = run_command(&cp); >> + if (status) >> + exit(status); >> +} > > This doesn't propagate the parent argv at all. So if I run: > > git ls-files -z --recurse-submodules > > then the paths are all NUL-terminated in the parent, but > newline-terminated in the submodules. Oops. Yep definitely missed that. I can fix that. I think the main reason for not blindly copying the argv array is that there may be some things we don't want to pass to the child. While not in the context of ls-files, I was working on recursive grep earlier and with that you can pass a rev to grep. You can't blindly copy that because the rev is meaningless to the the child and may produce broken output. Instead we would need to pass the actual rev of what the parent has checked out in that particular rev. I haven't thought it completely through yet but it did discourage me from blindly copying the args across. -Brandon