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

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 13, 2020, 16:51 UTC
Message-ID: <xmqqimfuzz73.fsf@gitster.c.googlers.com>
In-Reply-To: <20200613142306.GA4680@danh.dev>

Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:

Show 10 quoted lines

>>  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.