On Mon, Oct 23, 2017 at 2:29 PM, Jonathan Nieder <jrnieder@xxxxxxxxx> wrote:
> The git_connect function is growing long. Split the
> PROTO_GIT-specific portion to a separate function to make it easier to
> read.
>
> No functional change intended.
>
> Signed-off-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
This also looks good to me.
unrelated:
Patch 2 was very easy to review using "log -p -w --color-moved",
this one however was not. This is because -w caused the diff machinery
to generate a completely different diff. (Not showing the new function
completely but some weird function header trickery. The white space
mangled output is below; most of it was colored "moved")
I had to have -w as otherwise --color-moved would not work,
so maybe we want to have an option to ignore white space for the
sake of move detection only, not affecting the diff in general;
maybe '--ignore-white-space-in-move-detection'?
I think once this option is given, all we have to do is pay attention to
this option in diff.c#moved_entry_cmp/next_byte, which is best built
on top of Peffs recent fixes origin/jk/diff-color-moved-fix.
Would that be of interest for people?
Thanks,
Stefan
diff --git a/connect.c b/connect.c
index 7fbd396b35..068e70caad 100644
--- a/connect.c
+++ b/connect.c
@@ -851,36 +851,16 @@ static enum ssh_variant
determine_ssh_variant(const char *ssh_command,
}
/*
- * This returns a dummy child_process if the transport protocol does not
- * need fork(2), or a struct child_process object if it does. Once done,
- * finish the connection with finish_connect() with the value returned from
- * this function (it is safe to call finish_connect() with NULL to support
- * the former case).
+ * Open a connection using Git's native protocol.
*
- * If it returns, the connect is successful; it just dies on errors (this
- * will hopefully be changed in a libification effort, to return NULL when
- * the connection failed).
+ * The caller is responsible for freeing hostandport, but this function may
+ * modify it (for example, to truncate it to remove the port part).
*/
-struct child_process *git_connect(int fd[2], const char *url,
- const char *prog, int flags)
+static struct child_process *git_connect_git(int fd[2], char *hostandport,
+ const char *path, const
char *prog,
+ int flags)
{
- char *hostandport, *path;
struct child_process *conn = &no_fork;
- enum protocol protocol;
-
- /* Without this we cannot rely on waitpid() to tell
- * what happened to our children.
- */
- signal(SIGCHLD, SIG_DFL);
-
- protocol = parse_connect_url(url, &hostandport, &path);
- if ((flags & CONNECT_DIAG_URL) && (protocol != PROTO_SSH)) {
- printf("Diag: url=%s\n", url ? url : "NULL");
- printf("Diag: protocol=%s\n", prot_name(protocol));
- printf("Diag: hostandport=%s\n", hostandport ?
hostandport : "NULL");
- printf("Diag: path=%s\n", path ? path : "NULL");
- conn = NULL;
- } else if (protocol == PROTO_GIT) {
struct strbuf request = STRBUF_INIT;
/*
* Set up virtual host information based on where we will
@@ -925,6 +905,41 @@ struct child_process *git_connect(int fd[2],
const char *url,
free(target_host);
strbuf_release(&request);
+ return conn;
+}
+
+/*
+ * This returns a dummy child_process if the transport protocol does not
+ * need fork(2), or a struct child_process object if it does. Once done,
+ * finish the connection with finish_connect() with the value returned from
+ * this function (it is safe to call finish_connect() with NULL to support
+ * the former case).
+ *
+ * If it returns, the connect is successful; it just dies on errors (this
+ * will hopefully be changed in a libification effort, to return NULL when
+ * the connection failed).
+ */
+struct child_process *git_connect(int fd[2], const char *url,
+ const char *prog, int flags)
+{
+ char *hostandport, *path;
+ struct child_process *conn = &no_fork;
+ enum protocol protocol;
+
+ /* Without this we cannot rely on waitpid() to tell
+ * what happened to our children.
+ */
+ signal(SIGCHLD, SIG_DFL);
+
+ protocol = parse_connect_url(url, &hostandport, &path);
+ if ((flags & CONNECT_DIAG_URL) && (protocol != PROTO_SSH)) {
+ printf("Diag: url=%s\n", url ? url : "NULL");
+ printf("Diag: protocol=%s\n", prot_name(protocol));
+ printf("Diag: hostandport=%s\n", hostandport ?
hostandport : "NULL");
+ printf("Diag: path=%s\n", path ? path : "NULL");
+ conn = NULL;
+ } else if (protocol == PROTO_GIT) {
+ conn = git_connect_git(fd, hostandport, path, prog, flags);
} else {
struct strbuf cmd = STRBUF_INIT;
const char *const *var;
