Re: [PATCH v8 2/4] receive-pack: drop static variables to track report status version

[Junio C Hamano <gitster@pobox.com>]

Karthik Nayak <karthik.188@gmail.com> writes:

> @@ -2563,7 +2569,7 @@ int cmd_receive_pack(int argc,
>  			   PACKET_READ_CHOMP_NEWLINE |
>  			   PACKET_READ_DIE_ON_ERR_PACKET);
>  
> -	if ((commands = read_head_info(&reader, &shallow))) {
> +	if ((commands = read_head_info(&reader, &shallow, &version))) {
>  		struct string_list push_options = STRING_LIST_INIT_DUP;
>  		struct strbuf unpack_status = STRBUF_INIT;
>  
> @@ -2596,10 +2602,18 @@ int cmd_receive_pack(int argc,
>  				 &push_options);
>  		odb_transaction_finalize(transaction);
>  		sigchain_push(SIGPIPE, SIG_IGN);
> -		if (report_status_v2)
> +
> +		switch (version) {
> +		case REPORT_STATUS_V2:
>  			report_v2(commands, &unpack_status);
> -		else if (report_status)
> +			break;
> +		case REPORT_STATUS_V0:
>  			report(commands, &unpack_status);
> +			break;
> +		default:
> +			BUG("unknown report status version");
> +		}

Sorry that I should have noticed earlier, but is this really what we
want?  version is read by read_head_info() from the other side, and
in earlier iterations of this series we used to have something like

	if (report_status_v2)
		report_v2(...);
	else if (report_status)
		report(...);

without "else die()".

In any case, BUG() here is inappropriate, as setting "version" to v0
or v2 is totally up to what the other side of the connection would
say.  BUG() is about a programming error in our code, on _this_ end
of the connection.

We probably should have

		case REPORT_STATUS_UNKNOWN:
			break;

to catch the case where the other side did not ask any report-status
and do nothing about it.