Re: [WIP 08/15] connect: discover protocol version outside of get_remote_heads
flat view
From: Junio C Hamano <hidden>
Date: 2017-12-07 18:51:06
Brandon Williams [off-list ref] writes:
quoted hunk
@@ -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?