From: Junio C Hamano <gitster@pobox.com>
Date: Wed, 03 Aug 2016 20:18:55 GMT
Subject: Re: [PATCH v4 01/12] pkt-line: extract set_packet_header()
Message-ID: <xmqqd1lp8v2o.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20160803164225.46355-2-larsxschneider@gmail.com>

larsxschneider@gmail.com writes:

> From: Lars Schneider <larsxschneider@gmail.com>
>
> set_packet_header() converts an integer to a 4 byte hex string. Make
> this function locally available so that other pkt-line functions can
> use it.

Didn't I say that this is a bad idea already in an earlier review?

The only reason why you want it, together with direct_packet_write()
(which I think is another bad idea), is because you use
packet_buf_write() to create a "<header><payload>" in a buf in the
usercode in step 11/12 like this:

+	packet_buf_write(&nbuf, "command=%s\n", filter_type);
+	ret = !direct_packet_write(process->in, nbuf.buf, nbuf.len, 1);

which would be totally unnecessary if you just did strbuf_addf()
into nbuf and used packet_write() like everybody else does.

Puzzled.  Why are steps 01/12 and 02/12 an improvement?