Brandon Williams <bmw...@google.com> writes:
> @@ -193,7 +195,17 @@ int cmd_fetch_pack(int argc, const char **argv, const
> char *prefix)
> if (!conn)
> return args.diag_url ? 0 : 1;
> }
> - get_remote_heads(fd[0], NULL, 0, &ref, 0, NULL, &shallow);
> +
> + packet_reader_init(&reader, fd[0], NULL, 0);
> +
> + switch (discover_version(&reader)) {
> + case protocol_v1:
> + case protocol_v0:
> + get_remote_heads(&reader, &ref, 0, NULL, &shallow);
> + break;
> + case protocol_unknown_version:
> + BUG("unknown protocol version");
> + }
We see quite a few hunks just like this one appear in this patch.
The repetition is a bit disturbing. I wonder if we want a wrapper
around the "reader-init && discover-version && return an error if
the protocol version is not known" dance. That would allow us to
write this part of the code like so:
struct packet_reader reader;
if (connection_preamble(&reader, fd[0]))
die("unknown protocol version");
get_remote_heads(&reader, &ref, 0, NULL, &shallow);
or something like that.
By the way, is that really a BUG()? Getting a connection and the
other end declaring a protocol version you do not yet understand is
not a bug in your program---it is a normal runtime error, no?
