Re: [PATCH v5 05/15] pkt-line: add packet_write_gently_fmt()

Jeff King [off-list ref] writes:

On Wed, Aug 10, 2016 at 03:04:01PM +0200, larsxschneider@gmail.com wrote:

quoted

+int packet_write_gently_fmt(int fd, const char *fmt, ...)
+{
+	static struct strbuf buf = STRBUF_INIT;
+	va_list args;
+
+	strbuf_reset(&buf);
+	va_start(args, fmt);
+	format_packet(1, &buf, fmt, args);
+	va_end(args);
+	packet_trace(buf.buf + 4, buf.len - 4, 1);
+	return (write_in_full(fd, buf.buf, buf.len) == buf.len ? 0 : -1);
+}

Could the end of this function just be:

  return packet_write_gently(fd, buf.buf, buf.len);

? I guess we'd prefer to avoid that, because it incurs an extra
memmove() of the data.

Similarly, I'd think this could share code with the non-gentle form
(which should be able to just call this and die() if returns an error).
Though sometimes the va_list transformation makes that awkward.

Yes.

Also regarding the naming, please have "_gently" at the end; that is
how all other function families with _gently variant are named, I
think.

On 10 Aug 2016, at 19:18, Junio C Hamano [off-list ref] wrote:

Jeff King [off-list ref] writes:

quoted

On Wed, Aug 10, 2016 at 03:04:01PM +0200, larsxschneider@gmail.com wrote:

quoted

+int packet_write_gently_fmt(int fd, const char *fmt, ...)
+{
+	static struct strbuf buf = STRBUF_INIT;
+	va_list args;
+
+	strbuf_reset(&buf);
+	va_start(args, fmt);
+	format_packet(1, &buf, fmt, args);
+	va_end(args);
+	packet_trace(buf.buf + 4, buf.len - 4, 1);
+	return (write_in_full(fd, buf.buf, buf.len) == buf.len ? 0 : -1);
+}

Could the end of this function just be:

 return packet_write_gently(fd, buf.buf, buf.len);

? I guess we'd prefer to avoid that, because it incurs an extra
memmove() of the data.

Similarly, I'd think this could share code with the non-gentle form
(which should be able to just call this and die() if returns an error).
Though sometimes the va_list transformation makes that awkward.

Yes.

Peff just posted that he tried the shared code idea but the result
ended up ugly.

Also regarding the naming, please have "_gently" at the end; that is
how all other function families with _gently variant are named, I
think.

OK, I will rename them.

Thanks,
Lars