From owner-svn-src-all@FreeBSD.ORG Fri Feb 27 10:28:41 2015 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id AEA72A6A; Fri, 27 Feb 2015 10:28:41 +0000 (UTC) Received: from mail104.syd.optusnet.com.au (mail104.syd.optusnet.com.au [211.29.132.246]) by mx1.freebsd.org (Postfix) with ESMTP id 701966BD; Fri, 27 Feb 2015 10:28:40 +0000 (UTC) Received: from c211-30-166-197.carlnfd1.nsw.optusnet.com.au (c211-30-166-197.carlnfd1.nsw.optusnet.com.au [211.30.166.197]) by mail104.syd.optusnet.com.au (Postfix) with ESMTPS id C1C54420C3A; Fri, 27 Feb 2015 20:56:39 +1100 (AEDT) Date: Fri, 27 Feb 2015 20:56:39 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Andrew Turner Subject: Re: svn commit: r279349 - head/sys/kern In-Reply-To: <20150227082257.3fb1081c@bender.Home> Message-ID: <20150227202646.I2088@besplex.bde.org> References: <201502270256.t1R2uxnv085328@svn.freebsd.org> <20150227082257.3fb1081c@bender.Home> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.1 cv=Za4kaKlA c=1 sm=1 tr=0 a=KA6XNC2GZCFrdESI5ZmdjQ==:117 a=PO7r1zJSAAAA:8 a=kj9zAlcOel0A:10 a=JzwRw_2MAAAA:8 a=6I5d2MoRAAAA:8 a=SPxCmRf2tiERg1a4eD0A:9 a=CjuIK1q_8ugA:10 Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Warner Losh X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 27 Feb 2015 10:28:41 -0000 On Fri, 27 Feb 2015, Andrew Turner wrote: > On Fri, 27 Feb 2015 02:56:59 +0000 (UTC) > Warner Losh wrote: > ... >> /* >> + * We need some randomness. Implement the classic Linear Congruential >> + * generator X_{n+1}=(aX_n+c) mod m. These values are optimized for >> + * m = 2^32, a = 69069 and c = 5. This is signed so that we can get >> + * both positive and negative values from it by shifting the value >> + * right. >> + */ >> +static int sched_random() >> +{ >> + int rnd, *rndptr; >> + rndptr = DPCPU_PTR(randomval); >> + rnd = *rndptr * 69069 + 5; >> + *rndptr = rnd; >> + return(rnd); >> +} > > Didn't we recently have issues with signed integer overflow being > undefined? Even though we worked around it with a compiler flag it > would be better to not rely on undefined behaviour in the first place. It just moves bad code from inside 1 function, adds mounds of style bugs, (about 10 style bugs of 6 different types in the above; a few more elsewhere) and calls the bad code from where it was moved from and another function. The undefined behaviour is missing in old rand() in libc. That uses unsigned long internally to avoid the undefined behaviour and to not depend on ints being 32 bits, but returns only 15 bits so that the value can be represented as a (nonnegative) 16-bit int. Normally, LCGs have a large multiplier that puts most randomness in the top bits so lower bits should be discarded. This one does the opposite. Here the comment can be read as admitting that the undefined behaviour is intentional. "This" in the comment is ambiguous. I think it means the return value. The result of right shifting a negative int is only implementation-defined. Bruce