Re: [PATCH v7 04/10] pkt-line: add packet_flush_gently()

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 12, 2016, 23:30 UTC
Message-ID: <xmqqinu0y9v1.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20160908182132.50788-5-larsxschneider@gmail.com>

larsxschneider@gmail.com writes:

Show 14 quoted lines

> From: Lars Schneider <larsxschneider@gmail.com>
>
> packet_flush() would die in case of a write error even though for some
> callers an error would be acceptable. Add packet_flush_gently() which
> writes a pkt-line flush packet and returns `0` for success and `-1` for
> failure.
> ...
> +int packet_flush_gently(int fd)
> +{
> +	packet_trace("0000", 4, 1);
> +	if (write_in_full(fd, "0000", 4) == 4)
> +		return 0;
> +	error("flush packet write failed");
> +	return -1;

It is more idiomatic to do

	return error(...);

but more importantly, does the caller even want an error message unconditionally printed here?

I suspect that it is a strong sign that the caller wants to be in control of when and what error message is produced; otherwise it wouldn't be calling the _gently() variant, no?

Of course, if you have written callers to this function in later patches in this series, they would be responsible for reporting (or choosing not to report) this failure, but I think making this function silent is a better course in the longer term.