Re: [PATCH v6 03/13] pkt-line: add packet_write_fmt_gently()

> On 25 Aug 2016, at 23:41, Junio C Hamano <gitster@pobox.com> wrote:
> 
> larsxschneider@gmail.com writes:
> 
>> From: Lars Schneider <larsxschneider@gmail.com>
>> 
>> packet_write_fmt() would die in case of a write error even though for
>> some callers an error would be acceptable. Add packet_write_fmt_gently()
>> which writes a formatted pkt-line and returns `0` for success and `-1`
>> for an error.
>> 
>> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
>> ---
>> pkt-line.c | 12 ++++++++++++
>> pkt-line.h |  1 +
>> 2 files changed, 13 insertions(+)
>> 
>> diff --git a/pkt-line.c b/pkt-line.c
>> index e8adc0f..3e8b2fb 100644
>> --- a/pkt-line.c
>> +++ b/pkt-line.c
>> @@ -137,6 +137,18 @@ void packet_write_fmt(int fd, const char *fmt, ...)
>> 	write_or_die(fd, buf.buf, buf.len);
>> }
>> 
>> +int packet_write_fmt_gently(int fd, const char *fmt, ...)
>> +{
>> +	static struct strbuf buf = STRBUF_INIT;
>> +	va_list args;
>> +
>> +	strbuf_reset(&buf);
>> +	va_start(args, fmt);
>> +	format_packet(&buf, fmt, args);
>> +	va_end(args);
>> +	return (write_in_full(fd, buf.buf, buf.len) == buf.len ? 0 : -1);
>> +}
> 
> Even though its only a handful lines, it is a bit ugly to have a
> completely copied implementation only to have _gently().  I suspect
> that you should be able to
> 
> 	static int packet_write_fmt_1(int fd, int gently,
> 					const char *fmt, va_list args)
>        {
> 		struct strbuf buf = STRBUF_INIT;
> 		size_t count;
> 
> 		format_packet(&buf, fmt, args);
> 		
> 		count = write_in_full(fd, buf.buf, buf.len);
>                if (count == buf.len)
>                	return 0;
> 		if (!gently) {
> 			check_pipe(errno);
>                	die_errno("write error");
> 		}
>                return -1;
> 	}
> 
> and then share that between the existing one:
> 
> 	void packet_write_fmt(int fd, const char *fmt, ...)
>        {
> 		va_list args;
> 	        va_start(args, fmt);
>                packet_write_fmt_1(fd, 0, fmt, args);
>                va_end(args);
> 	}
> 
> and the new one:
> 
> 	void packet_write_fmt_gently(int fd, const char *fmt, ...)
>        {
> 		int status;
> 		va_list args;
> 	        va_start(args, fmt);
>                status = packet_write_fmt_1(fd, 1, fmt, args);
>                va_end(args);
> 		return status;
> 	}