From: Jakub Narębski <jnareb@gmail.com>
Date: Wed, 03 Aug 2016 20:05:51 GMT
Subject: Re: [PATCH v3 01/10] pkt-line: extract set_packet_header()
Message-ID: <51080fb7-2bab-a100-0971-e82063c1ac78@gmail.com>
In-Reply-To: <73AED3FA-C666-4C49-90B2-387E410F7D52@gmail.com>

[This response might have been invalidated by v4]

W dniu 01.08.2016 o 13:33, Lars Schneider pisze: 
>> On 30 Jul 2016, at 12:30, Jakub Narębski <jnareb@gmail.com> wrote:

>>> #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
  }