> On 03 Aug 2016, at 22:05, Jakub Narębski <jnareb@xxxxxxxxx> wrote:
>
> [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@xxxxxxxxx> 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
> }
>
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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html