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

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 25, 2016, 21:41 UTC
Message-ID: <xmqq8tvkle6o.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20160825110752.31581-4-larsxschneider@gmail.com>

larsxschneider@gmail.com writes:

Show 32 quoted lines

> 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;
	}

Show 15 quoted lines

>  void packet_buf_write(struct strbuf *buf, const char *fmt, ...)
>  {
>  	va_list args;
> diff --git a/pkt-line.h b/pkt-line.h
> index 1902fb3..3caea77 100644
> --- a/pkt-line.h
> +++ b/pkt-line.h
> @@ -23,6 +23,7 @@ void packet_flush(int fd);
>  void packet_write_fmt(int fd, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
>  void packet_buf_flush(struct strbuf *buf);
>  void packet_buf_write(struct strbuf *buf, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
> +int packet_write_fmt_gently(int fd, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
>  
>  /*
>   * Read a packetized line into the buffer, which must be at least size bytes