{"thread":{"id":"47479","subject":"[PATCH] http: use internal argv_array of struct child_process","startedAt":"2017-12-22T08:14:14Z","lastAt":"2017-12-22T11:57:12Z","messageCount":3,"participants":["René Scharfe","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"335160","messageId":"504cafe3-2960-af2d-10fe-51e8ba3d2213@web.de","threadId":"47479","inReplyTo":null,"subject":"[PATCH] http: use internal argv_array of struct child_process","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-12-22T08:14:02Z","receivedAt":"2017-12-22T08:14:14Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Avoid a strangely magic array size (it's slightly too big) and explicit\nindex numbers by building the command line for index-pack using the\nembedded argv_array of the child_process. The resulting code is shorter\nand easier to extend.\n\nSigned-off-by: Rene Scharfe \n---\n http.c | 12 ++++--------\n 1 file changed, 4 insertions(+), 8 deletions(-)\n\ndiff --git a/http.c b/http.c\nindex 215bebef1b..b22b4ada58 100644\n--- a/http.c\n+++ b/http.c\n@@ -2025,7 +2025,6 @@ int finish_http_pack_request(struct http_pack_request *preq)\n \tchar *tmp_idx;\n \tsize_t len;\n \tstruct child_process ip = CHILD_PROCESS_INIT;\n-\tconst char *ip_argv[8];\n \n \tclose_pack_index(p);\n \n@@ -2041,13 +2040,10 @@ int finish_http_pack_request(struct http_pack_request *preq)\n \t\tdie(\"BUG: pack tmpfile does not end in .pack.temp?\");\n \ttmp_idx = xstrfmt(\"%.*s.idx.temp\", (int)len, preq->tmpfile);\n \n-\tip_argv[0] = \"index-pack\";\n-\tip_argv[1] = \"-o\";\n-\tip_argv[2] = tmp_idx;\n-\tip_argv[3] = preq->tmpfile;\n-\tip_argv[4] = NULL;\n-\n-\tip.argv = ip_argv;\n+\targv_array_push(&ip.args, \"index-pack\");\n+\targv_array_push(&ip.args, \"-o\");\n+\targv_array_push(&ip.args, tmp_idx);\n+\targv_array_push(&ip.args, preq->tmpfile);\n \tip.git_cmd = 1;\n \tip.no_stdin = 1;\n \tip.no_stdout = 1;\n-- \n2.15.1\n"},{"id":"335162","messageId":"CAPig+cQaQ9ae1e4eMfJMG4wykXe2tTgtvpxB1_Z=3XbitYeQyw@mail.gmail.com","threadId":"47479","inReplyTo":"504cafe3-2960-af2d-10fe-51e8ba3d2213@web.de","subject":"Re: [PATCH] http: use internal argv_array of struct child_process","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2017-12-22T08:59:52Z","receivedAt":"2017-12-22T09:54:46Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Dec 22, 2017 at 3:14 AM, René Scharfe wrote:\n> Avoid a strangely magic array size (it's slightly too big) and explicit\n> index numbers by building the command line for index-pack using the\n> embedded argv_array of the child_process. The resulting code is shorter\n> and easier to extend.\n>\n> Signed-off-by: Rene Scharfe \n> ---\n> diff --git a/http.c b/http.c\n> @@ -2041,13 +2040,10 @@ int finish_http_pack_request(struct http_pack_request *preq)\n> - ip_argv[0] = \"index-pack\";\n> - ip_argv[1] = \"-o\";\n> - ip_argv[2] = tmp_idx;\n> - ip_argv[3] = preq->tmpfile;\n> - ip_argv[4] = NULL;\n> -\n> - ip.argv = ip_argv;\n> + argv_array_push(&ip.args, \"index-pack\");\n> + argv_array_push(&ip.args, \"-o\");\n> + argv_array_push(&ip.args, tmp_idx);\n> + argv_array_push(&ip.args, preq->tmpfile);\n\nNot necessarily worth a re-roll, but using the \"pushl\" variant would\nmake it clear that \"-o\" and tmp_idx are related and would ensure that\nthey don't accidentally get split up if someone inserts a new \"push\"\nin the sequence in the future.\n\n argv_array_push(&ip.args, \"index-pack\");\n argv_array_pushl(&ip.args, \"-o\", tmp_idx, NULL);\n argv_array_push(&ip.args, preq->tmpfile);\n"},{"id":"335167","messageId":"6bebb8a0-31ec-b20c-2430-2d69daa63dea@web.de","threadId":"47479","inReplyTo":"CAPig+cQaQ9ae1e4eMfJMG4wykXe2tTgtvpxB1_Z=3XbitYeQyw@mail.gmail.com","subject":"[PATCH v2] http: use internal argv_array of struct child_process","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-12-22T11:56:59Z","receivedAt":"2017-12-22T11:57:12Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Avoid a strangely magic array size (it's slightly too big) and explicit\nindex numbers by building the command line for index-pack using the\nembedded argv_array of the child_process. Add the flag -o and its\nargument with argv_array_pushl() to make it obvious that they belong\ntogether. The resulting code is shorter and easier to extend.\n\nHelped-by: Eric Sunshine \nSigned-off-by: Rene Scharfe \n---\nChange from v1: use argv_array_pushl for -o and tmp_idx.\n\nThanks, Eric, good idea!\n\n http.c | 11 +++--------\n 1 file changed, 3 insertions(+), 8 deletions(-)\n\ndiff --git a/http.c b/http.c\nindex 215bebef1b..9f2fcc9ec4 100644\n--- a/http.c\n+++ b/http.c\n@@ -2025,7 +2025,6 @@ int finish_http_pack_request(struct http_pack_request *preq)\n \tchar *tmp_idx;\n \tsize_t len;\n \tstruct child_process ip = CHILD_PROCESS_INIT;\n-\tconst char *ip_argv[8];\n \n \tclose_pack_index(p);\n \n@@ -2041,13 +2040,9 @@ int finish_http_pack_request(struct http_pack_request *preq)\n \t\tdie(\"BUG: pack tmpfile does not end in .pack.temp?\");\n \ttmp_idx = xstrfmt(\"%.*s.idx.temp\", (int)len, preq->tmpfile);\n \n-\tip_argv[0] = \"index-pack\";\n-\tip_argv[1] = \"-o\";\n-\tip_argv[2] = tmp_idx;\n-\tip_argv[3] = preq->tmpfile;\n-\tip_argv[4] = NULL;\n-\n-\tip.argv = ip_argv;\n+\targv_array_push(&ip.args, \"index-pack\");\n+\targv_array_pushl(&ip.args, \"-o\", tmp_idx, NULL);\n+\targv_array_push(&ip.args, preq->tmpfile);\n \tip.git_cmd = 1;\n \tip.no_stdin = 1;\n \tip.no_stdout = 1;\n-- \n2.15.1\n"}]}