Re: [PATCH v2] pkt-line: use string versions of functions

From: Junio C Hamano <gitster@pobox.com>
To: Denton Liu <liu.denton@gmail.com>
Cc: "Git Mailing List" <git@vger.kernel.org>,
	"Jeff King" <peff@peff.net>,
	"Đoàn Trần Công Danh" <congdanhqx@gmail.com>
Subject: Re: [PATCH v2] pkt-line: use string versions of functions
Date: Sun, 14 Jun 2020 14:35:06 -0700	[thread overview]
Message-ID: <xmqqeeqhxred.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <da9ba5fb2d0fb9481f81ce525cbabaedd722858d.1592125442.git.liu.denton@gmail.com> (Denton Liu's message of "Sun, 14 Jun 2020 05:07:54 -0400")

Denton Liu <liu.denton@gmail.com> writes:

> +#define control_packet_write(fd, s, errstr) \
> +	do { \
> +		(void)s"is a string constant"; \
> +		packet_trace_str((s), 1); \
> +		if (write_str_in_full((fd), (s)) < 0) \
> +			die_errno((errstr)); \
> +	} while (0)
> +

Oh, that's much better.  If you go this route, drop your use of
write_str_in_full(), but count the length of s with strlen() here
to give the chance to the compilers to count the constant strings
at compile time.

>  /*
>   * If we buffered things up above (we don't, but we should),
>   * we'd flush it here
>   */
>  void packet_flush(int fd)
>  {
> -	packet_trace("0000", 4, 1);
> -	if (write_in_full(fd, "0000", 4) < 0)
> -		die_errno(_("unable to write flush packet"));
> +	control_packet_write(fd, "0000", _("unable to write flush packet"));

> +#define control_packet_buf_write(buf, s) \
> +	do { \
> +		(void)s"is a string constant"; \
> +		packet_trace_str((s), 1); \
> +		strbuf_addstr((buf), (s)); \
> +	} while (0)
> +

Likewise for strbuf_addstr().

>  void packet_buf_flush(struct strbuf *buf)
>  {
> -	packet_trace("0000", 4, 1);
> -	strbuf_add(buf, "0000", 4);
> +	control_packet_buf_write(buf, "0000");
>  }