Junio C Hamano schrieb:
Johannes Sixt <johannes.sixt@xxxxxxxxxx> writes:
This prepares the API of git_connect() and finish_connect() to operate on
a struct child_process. Currently, we just use that object as a placeholder
for the pid that we used to return. A follow-up patch will change the
implementation of git_connect() and finish_connect() to make full use
of the object.
Good description, except removal of checks for negative return
of the calling sites raised my eyebrow and had me spend a few
more minutes to review than necessary (see below).
I've thought about this issue a bit more.
Letting git_connect() die on error unconditionally is poison for any
libification efforts. So here's a plan:
1. Let git_connect() return a struct child_process even for the
non-forking case. This way a return value of NULL can uniquely
identify a failure.
2. Keep the error checks in the callers (adjust to test for NULL).
3. Change the die() calls to return failure.
4. Note that the int fd[2] parameter to git_connect() is really an
output: Remove it and use .in and .out of the returned struct
child_process instead.
And maybe:
5. Reuse somehow the struct child_process that git_proxy_connect()
already fills in.
Since my patch doesn't do (1), it can't do (2), i.e. keep the error checks -
they must be removed because no unique failure value exists. So I could
complete (1) in a new version of this patch, in order to also do (2). What
is your preference?
-- Hannes
PS: I've postponed the completion of this plan - in favor of the MinGW port
integration - because it only helps libification.
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
