Brandon Williams <bmwill@xxxxxxxxxx> writes:
> +/* Returns 1 if packet_buffer is a protocol version pkt-line, 0 otherwise. */
> +static int process_protocol_version(void)
> +{
> + switch (determine_protocol_version_client(packet_buffer)) {
> + case protocol_v1:
> + return 1;
> + case protocol_v0:
> + return 0;
> + default:
> + die("server is speaking an unknown protocol");
> + }
> +}
For the purpose of "technology demonstration" v1 protocol, it is OK
to discard the result of "determine_pvc()" like the above code, but
in a real application, we would do a bit more than just ignoring an
extra "version #" packet that appears at the beginning, no?
It would be sensible to design how the result of determien_pvc()
call is propagated to the remainder of the program in this patch and
implement it. Perhaps add a new global (like server_capabilities
already is) and store the value there, or something? Or pass a
pointer to enum protocol_version as a return-location parameter to
this helper function so that the process_capabilities() can pass a
pointer to its local variable?
> static void process_capabilities(int *len)
> {
> @@ -224,12 +239,19 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
> */
> int responded = 0;
> int len;
> - int state = EXPECTING_FIRST_REF;
> + int state = EXPECTING_PROTOCOL_VERSION;
>
> *list = NULL;
>
> while ((len = read_remote_ref(in, &src_buf, &src_len, &responded))) {
> switch (state) {
> + case EXPECTING_PROTOCOL_VERSION:
> + if (process_protocol_version()) {
> + state = EXPECTING_FIRST_REF;
> + break;
> + }
> + state = EXPECTING_FIRST_REF;
> + /* fallthrough */
> case EXPECTING_FIRST_REF:
> process_capabilities(&len);
> if (process_dummy_ref()) {
