Ilari Liusvaara wrote:
> --- a/transport-helper.c
> +++ b/transport-helper.c
> @@ -862,3 +862,327 @@ int transport_helper_init(struct transport *transport, const char *name)
[...]
> +/* Unidirectional transfer. */
> +struct unidirectional_transfer
> +{
> + int src;
> + int dest;
> + int dest_is_sock;
> + int state;
> + char buf[BUFFERSIZE];
> + size_t bufuse;
> + const char *src_name;
> + const char *dest_name;
> +};
Nice. :)
(I would have written
int src;
int dest
unsigned dest_is_sock:1;
enum unidirectional_transfer_state state;
char buf[BUFFERSIZE];
...
to make the types more obvious, but I think that's more of a matter of
style.)
[...]
> +/* State of bidirectional transfer loop. */
> +struct bidirectional_transfer_state
> +{
> + struct pollfd polls[4];
> + int polls_active;
> + int stdin_index;
> + int stdout_index;
> + int input_index;
> + int output_index;
> + /* Direction from program to git. */
> + struct unidirectional_transfer ptg;
> + /* Direction from git to program. */
> + struct unidirectional_transfer gtp;
> +};
Sensible.
> +int bidirectional_transfer_loop(int input, int output)
> +{
> + struct bidirectional_transfer_state state;
> +
> + /* Fill the state fields. */
> + state.ptg.src = input;
> + state.ptg.dest = 1;
> + state.ptg.dest_is_sock = 0;
> + state.ptg.state = SSTATE_TRANSFERING;
> + state.ptg.bufuse = 0;
> + state.ptg.src_name = "remote input";
> + state.ptg.dest_name = "stdout";
> +
> + state.gtp.src = 0;
> + state.gtp.dest = output;
> + state.gtp.dest_is_sock = (input == output);
> + state.gtp.state = SSTATE_TRANSFERING;
> + state.gtp.bufuse = 0;
> + state.gtp.src_name = "stdin";
> + state.gtp.dest_name = "remote output";
> +
> + while (1) {
> + int r;
> + load_poll_params(&state);
> + if (!state.polls_active) {
> + transfer_debug("Transfer done");
> + break;
> + }
> + transfer_debug("Waiting for %i file descriptors",
> + state.polls_active);
> + r = poll(state.polls, state.polls_active, -1);
> + if (r < 0) {
> + if (errno == EWOULDBLOCK || errno == EAGAIN ||
> + errno == EINTR)
> + continue;
> + error("poll failed: %s", strerror(errno));
> + return -1;
> + } else if (r == 0)
> + continue;
> +
> + r = transfer_handle_events(&state);
> + if (r)
> + return r;
> + }
> + return 0;
> +}
Yes, that's much clearer.
Linux's scripts/checkpatch.pl would have a few things to say, but
I have no complaint myself except the density of comments (which is a
matter of taste).
Thanks!
--
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