Re: [PATCH v3 01/10] pkt-line: extract set_packet_header()

On 03 Aug 2016, at 22:05, Jakub Narębski [off-list ref] wrote:

[This response might have been invalidated by v4]

W dniu 01.08.2016 o 13:33, Lars Schneider pisze: 

quoted

quoted

On 30 Jul 2016, at 12:30, Jakub Narębski [off-list ref] wrote:

quoted

quoted

quoted

#define hex(a) (hexchar[(a) & 15])

I guess that this is inherited from the original, but this preprocessor
macro is local to the format_header() / set_packet_header() function,
and would not work outside it.  Therefore I think we should #undef it
after set_packet_header(), just in case somebody mistakes it for
a generic hex() function.  Perhaps even put it inside set_packet_header(),
together with #undef.

But I might be mistaken... let's check... no, it isn't used outside it.

Agreed. Would that be OK?

static void set_packet_header(char *buf, const int size)
{
	static char hexchar[] = "0123456789abcdef";
	#define hex(a) (hexchar[(a) & 15])
	buf[0] = hex(size >> 12);
	buf[1] = hex(size >> 8);
	buf[2] = hex(size >> 4);
	buf[3] = hex(size);
	#undef hex
}

That's better, though I wonder if we need to start #defines at begining
of line.  But I think current proposal is O.K.


Either this (which has unnecessary larger scope)

 #define hex(a) (hexchar[(a) & 15])
 static void set_packet_header(char *buf, const int size)
 {
 	static char hexchar[] = "0123456789abcdef";

 	buf[0] = hex(size >> 12);
 	buf[1] = hex(size >> 8);
 	buf[2] = hex(size >> 4);
 	buf[3] = hex(size);
 }
 #undef hex

or this (which looks worse)

 static void set_packet_header(char *buf, const int size)
 {
 	static char hexchar[] = "0123456789abcdef";
 #define hex(a) (hexchar[(a) & 15])
 	buf[0] = hex(size >> 12);
 	buf[1] = hex(size >> 8);
 	buf[2] = hex(size >> 4);
 	buf[3] = hex(size);
 #undef hex
 }

I probably will drop this patch as Junio is not convinced that it
is a good idea:
http://public-inbox.org/git/xmqqd1lp8v2o.fsf%40gitster.mtv.corp.google.com/

- Lars