Re: [PATCH 2/5] upload-archive: use regular "struct child_process" pattern
From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2021-11-22 20:56:01
On Mon, Nov 22 2021, Ævar Arnfjörð Bjarmason wrote:
quoted hunk ↗ jump to hunk
This pattern added [1] in seems to have been intentional, but since [2] and [3] we've wanted do initialization of what's now the "struct strvec" "args" and "env_array" members. Let's not trample on that initialization here. 1. 1bc01efed17 (upload-archive: use start_command instead of fork, 2011-11-19) 2. c460c0ecdca (run-command: store an optional argv_array, 2014-05-15) 3. 9a583dc39e (run-command: add env_array, an optional argv_array for env, 2014-10-19) Signed-off-by: Ævar Arnfjörð Bjarmason <redacted> --- builtin/upload-archive.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)diff --git a/builtin/upload-archive.c b/builtin/upload-archive.c index 24654b4c9bf..b4b9b3a6262 100644 --- a/builtin/upload-archive.c +++ b/builtin/upload-archive.c@@ -77,7 +77,7 @@ static ssize_t process_input(int child_fd, int band) int cmd_upload_archive(int argc, const char **argv, const char *prefix) { - struct child_process writer = { argv }; + struct child_process writer = CHILD_PROCESS_INIT; if (argc == 2 && !strcmp(argv[1], "-h")) usage(upload_archive_usage);@@ -92,6 +92,9 @@ int cmd_upload_archive(int argc, const char **argv, const char *prefix) argv[0] = "upload-archive--writer"; writer.out = writer.err = -1; writer.git_cmd = 1; + strvec_push(&writer.args, "upload-archive--writer"); + if (argc > 1) + strvec_pushv(&writer.args, &argv[1]); if (start_command(&writer)) { int err = errno; packet_write_fmt(1, "NACK unable to spawn subprocess\n");
Changing it to argv + 1 makes sense, I don't know why I did it like
that. It works, but is unusual.
But as to skipping the "argc > 1" test I've got this still:
@@ -89,9 +89,11 @@ int cmd_upload_archive(int argc, const char **argv, const char *prefix)
* multiplexed out to our fd#1. If the child dies, we tell the other
* end over channel #3.
*/
- argv[0] = "upload-archive--writer";
writer.out = writer.err = -1;
writer.git_cmd = 1;
+ strvec_push(&writer.args, "upload-archive--writer");
+ if (argc > 1)
+ strvec_pushv(&writer.args, argv + 1);
if (start_command(&writer)) {
int err = errno;
packet_write_fmt(1, "NACK unable to spawn subprocess\n");
We'll segfault if we give NULL to strvec_pushv() so we still need that
check. Were you thinking of strvec_pushl(), or am I missing something?
(Even with strvec_pushl() we can't interpolate it as-is into the list,
since it'll point to uninitialized memory in this case).