Re: CVS commit: src/lib/libc/stdlib

[Valery Ushakov <uwe%stderr.spb.ru@localhost>]

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