Subject: Re: [PATCH] pkt-line: extract out PACKET_HEADER_SIZE

From: Junio C Hamano <[email-masked]>
Date: Sat, 13 Jun 2020 09:51:28 -0700

Đoàn Trần Công Danh  <[email-masked]> writes:

>>  void packet_flush(int fd)
>>  {
>> -	packet_trace("0000", 4, 1);
>> -	if (write_in_full(fd, "0000", 4) < 0)
>> +	packet_trace("0000", PACKET_HEADER_SIZE, 1);
>> +	if (write_in_full(fd, "0000", PACKET_HEADER_SIZE) < 0)
>
> I think the magic number 4 is easier to follow than some macro
> PACKET_HEADER_SIZE defined elsewhere. Because reader may judge that is
> the size of "0000"

I hate to admit it immensely, but I tend to agree with you.  In the
context of this single call,

	write_in_full(fd, "0000", 4)
	write_in_full(fd, "0000", PACKET_HEADER_SIZE)

I find the former vastly easier.

> How about (this ugly code):
>
> 	packet_trace("0000", sizeof "0000" - 1, 1);
> 	if (write_in_full(fd, "0000", sizeof "0000" - 1) < 0)

Yeah, that is ugly.  I was thinking more in the direction of
replacing these three-argument write_in_full with something like

#define write_constant(fd, constant_string) \
	write_in_full((fd), (constant_string), strlen(constant_string))

with some preprocessor magic to make the compilation break when the
second parameter to the macro is not a string constant.