Re: [PATCH v9 10/14] pkt-line: add functions to read/write flush terminated packet streams

[Lars Schneider <larsxschneider@gmail.com>]

> On 04 Oct 2016, at 21:53, Junio C Hamano <gitster@pobox.com> wrote:
> 
> larsxschneider@gmail.com writes:
> 
>> From: Lars Schneider <larsxschneider@gmail.com>
>> 
> 
>> +int write_packetized_from_buf(const char *src_in, size_t len, int fd_out)
>> +{
>> +	static char buf[LARGE_PACKET_DATA_MAX];
>> +	int err = 0;
>> +	size_t bytes_written = 0;
>> +	size_t bytes_to_write;
>> +
>> +	while (!err) {
>> +		if ((len - bytes_written) > sizeof(buf))
>> +			bytes_to_write = sizeof(buf);
>> +		else
>> +			bytes_to_write = len - bytes_written;
>> +		if (bytes_to_write == 0)
>> +			break;
>> +		err = packet_write_gently(fd_out, src_in + bytes_written, bytes_to_write);
>> +		bytes_written += bytes_to_write;
>> +	}
>> +	if (!err)
>> +		err = packet_flush_gently(fd_out);
>> +	return err;
>> +}
> 
> Hmph, what is buf[] used for, other than its sizeof() taken to yield
> a constant LARGE_PACKET_DATA_MAX?

Agreed. This is stupid. I will fix it.


>> 
>> +	for (;;) {
>> +		strbuf_grow(sb_out, LARGE_PACKET_DATA_MAX);
>> +		packet_len = packet_read(fd_in, NULL, NULL,
>> +			// TODO: explain + 1
> 
> No // C99 comment please.
> 
> And I agree that the +1 needs to be explained.

Oh. I did not send the very latest version :-(

Is this explanation OK?

+       strbuf_grow(sb_out, LARGE_PACKET_DATA_MAX);
+       packet_len = packet_read(fd_in, NULL, NULL,
+           /* strbuf_grow() above always allocates one extra byte to
+            * store a '\0' at the end of the string. packet_read()
+            * writes a '\0' extra byte at the end, too. Let it know
+            * that there is already room for the extra byte.
+            */
+           sb_out->buf + sb_out->len, LARGE_PACKET_DATA_MAX+1,
+           PACKET_READ_GENTLE_ON_EOF);


Thanks,
Lars