@matheusbsbs / matheus-bagliano / commits / 1823bea10f

git_connect: use argv_array

This avoids magic numbers when we allocate fixed-size argv arrays, and makes it more obvious that we are not overflowing. It is also the first step to fixing a memory leak. When git_connect returns a child_process struct, the argv array in the struct is dynamically allocated, but the individual strings are not (they are either owned elsewhere, or are freed). Later, in finish_connect, we free the array but leave the strings alone. This works for the child_process created by git_connect, but if we use transport_take_over, we may also end up with a child_process created by transport-helper's get_helper. In that case, the strings are freshly allocated, and we would want to free them. However, we have no idea in finish_connect which type we have. By consistently using run-command's internal argv-array, we do not have to worry about this issue at all; finish_command takes care of it for us, and we can drop our manual free entirely. Note that this actually makes the get_helper leak slightly worse; now we are leaking both the strings and the array. But when we adjust it in a future patch, that leak will go away entirely. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed May 15, 2014 at 04:34 UTC 1823bea10fceb371c7876e598d2413c85890cafc

1 file changed +10 -18

connect.c

+10 -18

@@ -534,22 +534,18 @@ static int git_use_proxy(const char *host)

534 static struct child_process *git_proxy_connect(int fd[2], char *host)

535 {

536 const char *port = STR(DEFAULT_GIT_PORT);

537 - const char **argv;

537 struct child_process *proxy;

538

539 get_host_and_port(&host, &port);

540

542 - argv = xmalloc(sizeof(*argv) * 4);

543 - argv[0] = git_proxy_command;

544 - argv[1] = host;

545 - argv[2] = port;

546 - argv[3] = NULL;

541 proxy = xcalloc(1, sizeof(*proxy));

548 - proxy->argv = argv;

542 + argv_array_push(&proxy->args, git_proxy_command);

543 + argv_array_push(&proxy->args, host);

544 + argv_array_push(&proxy->args, port);

545 proxy->in = -1;

546 proxy->out = -1;

547 if (start_command(proxy))

552 - die("cannot start proxy %s", argv[0]);

548 + die("cannot start proxy %s", git_proxy_command);

549 fd[0] = proxy->out; /* read from proxy stdout */

550 fd[1] = proxy->in; /* write to proxy stdin */

551 return proxy;

@@ -663,7 +659,6 @@ struct child_process *git_connect(int fd[2], const char *url,

659 char *hostandport, *path;

660 struct child_process *conn = &no_fork;

661 enum protocol protocol;

666 - const char **arg;

662 struct strbuf cmd = STRBUF_INIT;

663

664 /* Without this we cannot rely on waitpid() to tell

@@ -707,7 +702,6 @@ struct child_process *git_connect(int fd[2], const char *url,

702 sq_quote_buf(&cmd, path);

703

704 conn->in = conn->out = -1;

710 - conn->argv = arg = xcalloc(7, sizeof(*arg));

705 if (protocol == PROTO_SSH) {

706 const char *ssh = getenv("GIT_SSH");

707 int putty = ssh && strcasestr(ssh, "plink");

@@ -718,22 +712,21 @@ struct child_process *git_connect(int fd[2], const char *url,

712

713 if (!ssh) ssh = "ssh";

714

721 - *arg++ = ssh;

715 + argv_array_push(&conn->args, ssh);

716 if (putty && !strcasestr(ssh, "tortoiseplink"))

723 - *arg++ = "-batch";

717 + argv_array_push(&conn->args, "-batch");

718 if (port) {

719 /* P is for PuTTY, p is for OpenSSH */

726 - *arg++ = putty ? "-P" : "-p";

727 - *arg++ = port;

720 + argv_array_push(&conn->args, putty ? "-P" : "-p");

721 + argv_array_push(&conn->args, port);

722 }

729 - *arg++ = ssh_host;

723 + argv_array_push(&conn->args, ssh_host);

724 } else {

725 /* remove repo-local variables from the environment */

726 conn->env = local_repo_env;

727 conn->use_shell = 1;

728 }

735 - *arg++ = cmd.buf;

736 - *arg = NULL;

729 + argv_array_push(&conn->args, cmd.buf);

730

731 if (start_command(conn))

732 die("unable to fork");

@@ -759,7 +752,6 @@ int finish_connect(struct child_process *conn)

752 return 0;

753

754 code = finish_command(conn);

762 - free(conn->argv);

755 free(conn);

756 return code;

757 }