This is the mail archive of the mailing list for the glibc project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] __builtin_expect cleanup for iconvdata/*.c

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

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]