On 23.02.2020 03:20, Valery Ushakov wrote: > On Sun, Feb 23, 2020 at 02:51:49 +0100, Kamil Rytarowski wrote: > >> On 23.02.2020 02:29, Valery Ushakov wrote: >>> On Sat, Feb 22, 2020 at 14:07:57 +0000, Kamil Rytarowski wrote: >>> >>>> Module Name: src >>>> Committed By: kamil >>>> Date: Sat Feb 22 14:07:57 UTC 2020 >>>> >>>> Modified Files: >>>> src/lib/libc/stdlib: _rand48.c >>>> >>>> Log Message: >>>> Avoid undefined behavior in the rand48(3) implementation >>>> >>>> Instead of implicid promotion to signed int, >>>> explicitly cast the arguments to unsigned int. >>> >>> Please, please, please, pay at least some attention to what is going >>> on around the code you are changing. >>> >>> If there's already code in this function that does: >>> >>> accu = (unsigned long) __rand48_mult[0] * (unsigned long) xseed[0]; >>> >>> then keep it consistent and don't do casts to a different type >>> >>> accu += (unsigned int) __rand48_mult[0] * (unsigned int) xseed[2]; >> >> cast to unsigned long still works, but changes algorithm. My change was >> performed deliberately. On the other hand and according to local tests >> the end-result for unsigned long produces the same reults as cast to >> unsigned int and unsigned char so it does not matter. > > I cannot make sense of your answer. Does the cast to unsigned long > there change the algorithm or does it produce the same result? If it > produces the same result, then it should be used to be consistent with > the rest of the code (or the rest of the code changed to use unsigned > int). If it does change the result, there should be a comment > explaining it. > > -uwe >
Algorithm would be changed from calculating on 32bit numbers with signed integer overflows to an algorithm calculating on 64bit numbers. The __dorand48() function truncates the result to least significant 16bits only so it does not matter. I retained operations on 32bits avoiding changes of types for stylistic reasons.
signature.asc
Description: OpenPGP digital signature
