Re: [PATCH v4 03/12] pkt-line: (optionally) libify the packet readers

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 3, 2021, 19:53 UTC
Message-ID: <xmqqwnuohv4t.fsf@gitster.c.googlers.com>
In-Reply-To: <e05467def4e158a5f1cfa3aafffdb5c77097859a.1613598529.git.gitgitgadget@gmail.com>

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 8 quoted lines

> @@ -313,6 +316,8 @@ static int get_packet_data(int fd, char **src_buf, 
>  		if (options & PACKET_READ_GENTLE_ON_EOF)
>  			return -1;
>  
> +		if (options & PACKET_READ_NEVER_DIE)
> +			return error(_("the remote end hung up unexpectedly"));
>  		die(_("the remote end hung up unexpectedly"));
>  	}

This hunk treats READ_NEVER_DIE as a less quiet version of GENTRL_ON_EOF, i.e. the new flag allows to continue even after the "hung up unexpectedly" condition that usually causes the process to die..

Show 12 quoted lines

> @@ -355,12 +363,19 @@ enum packet_read_status packet_read_with_status(i
> ...
> -	if ((unsigned)len >= size)
> +	if ((unsigned)len >= size) {
> +		if (options & PACKET_READ_NEVER_DIE)
> +			return error(_("protocol error: bad line length %d"),
> +				     len);
>  		die(_("protocol error: bad line length %d"), len);
> +	}
>  
>  	if (get_packet_data(fd, src_buffer, src_len, buffer, len, options) < 0) {
>  		*pktlen = -1;

In the post-context of this hunk, there is this code:

	if ((options & PACKET_READ_DIE_ON_ERR_PACKET) &&
	    starts_with(buffer, "ERR "))
		die(_("remote error: %s"), buffer + 4);

	*pktlen = len;
	return PACKET_READ_NORMAL;

But here, there is no way to override the DIE_ON_ERR with READ_NEVER_DIE.

The asymmetry is somewhat annoying (i.e. if "if you do not want to die upon ERR, don't pass DIE_ON_ERR" could be a valid suggestion to the callers, then "if you do not want to die upon an unexpected hung-up, pass GENTLE_ON_EOF" would equally be valid suggestion), but I'll let it pass.

Show 18 quoted lines

> diff --git a/pkt-line.h b/pkt-line.h
> index a7149429ac35..2e472efaf2c5 100644
> --- a/pkt-line.h
> +++ b/pkt-line.h
> @@ -75,10 +75,14 @@ int write_packetized_from_buf_no_flush(const char *src_in, size_t len, int fd_ou
>   *
>   * If options contains PACKET_READ_DIE_ON_ERR_PACKET, it dies when it sees an
>   * ERR packet.
> + *
> + * With `PACKET_READ_NEVER_DIE`, no errors are allowed to trigger die() (except
> + * an ERR packet, when `PACKET_READ_DIE_ON_ERR_PACKET` is in effect).
>   */
>  #define PACKET_READ_GENTLE_ON_EOF     (1u<<0)
>  #define PACKET_READ_CHOMP_NEWLINE     (1u<<1)
>  #define PACKET_READ_DIE_ON_ERR_PACKET (1u<<2)
> +#define PACKET_READ_NEVER_DIE         (1u<<3)
>  int packet_read(int fd, char **src_buffer, size_t *src_len, char
>  		*buffer, unsigned size, int options);