[PATCH] __builtin_expect cleanup for iconvdata/*.c

Florian Weimer fweimer@redhat.com
Mon Sep 8 20:43:00 GMT 2014


On 09/08/2014 10:11 PM, Kalle Olavi Niemitalo wrote:

> Why keep the second __builtin_expect?

 > And the first one here.

The first patch only contains the changes which could not reasonably be 
automated with a short script.

>> -	      || (__builtin_expect (ch, 0x81) == 0xfe && ch2 > 0xa0))	      \
>> +	      || (__glibc_unlikely (ch == 0xfe && ch2 > 0xa0)))		      \

> Also, the ch2 > 0xa0 test wasn't inside
> __builtin_expect originally, but I don't know what the
> probabilities are.

Yes, I should probably undo this change.  I made it deliberately because 
I thought that the intent was to mark any reason to enter the error 
condition as unlikely, but then I'd have to wrap the entire if condition 
for consistency.

Note that is quite unlikely that current GCC uses __builtin_expect hints 
in the same way as it did when this code was initially written, and as 
I've seen during my binary diffing, the whole mechanism is quite brittle 
for such complex conditions.

-- 
Florian Weimer / Red Hat Product Security



More information about the Libc-alpha mailing list