2010/2/16 Vladimir 'φ-coder/phcoder' Serbinenko <phco...@gmail.com>: > Carles Pina i Estany wrote: >> Hello, >> >> The common pattern when doing a search by bisection is something like: >> + current = min + (max - min) / 2; >> >> Instead of the first natural idea: >> - current = (max + min) / 2; >> >> To avoid overflows. >> >> In gettext/gettext.c it's used in the "incorrect" way. It's not a big >> problem since would happen only with .mo files with lot of strings, like >> number that int represents in that architecture divided by 2 (aprox >> aprox.). >> >> See the attached file for a patch if we want to patch. >> > You forgot to make the same change before the loop. But actually it > doesn't matter because overflow in this statement: > internal_position = offsettranslation + position * 8; > is reached well before the overflow in bisecting. Anyway this > mattersonly for >4GiB files and unless we put videos in .po I don't see > how this limit would be reached. So I prefer readability and would > reject this patch.
There's a very readable way to express the "better" code that's immune to the overflow: int const range = max - min; /* or unsigned int, if min and max are declared that way */ current = min + range / 2; >> Else I would at least add a comment that we simplified because we >> consider that will not happen. >> >> Thanks, >> >> >> ------------------------------------------------------------------------ >> >> _______________________________________________ >> Grub-devel mailing list >> Grub-devel@gnu.org >> http://lists.gnu.org/mailman/listinfo/grub-devel > > > -- > Regards > Vladimir 'φ-coder/phcoder' Serbinenko > > > > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org > http://lists.gnu.org/mailman/listinfo/grub-devel > > _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
