Everything
php
Learn Community Releases The PHP Project
Home » Project » Newsgroups » php.internals » BC break with rand() where min > max »

BC break with rand() where min > max

php.internals

Leigh

10 years ago
Hi all, There has been an unforeseen break with rand() when the minimum value is greater than the maximum. Prior to 7.1 rand() would happily accept backwards parameters and return a value, however in the 7.1 branch it now emits a warning and returns false. I've preemptively committed a fix to allow min > max and return a value as in previous versions, but have kept the warning. Looking for some feedback/opinions on whether anyone else thinks this should be fixed differently (or not at all). N.B. this also changes the behaviour of mt_rand to now accept min > max

Davey

10 years ago
Leigh, I would _very_ much prefer to keep this BC. If we do the right thing, and there are no repercussions, I'd actually remove the warning. If you think we might want to disallow this in the future, use an E_DEPRECATED, but a warning seems pointless unless it's doing the wrong thing, or it's going to start doing the wrong thing at some point. - Davey On Mon, Aug 8, 2016 at 2:36 PM, Leigh <leight@gmail.com> wrote:

Christoph Becker

10 years ago
On 09.08.2016 at 00:12, Davey Shafik wrote:
> I would _very_ much prefer to keep this BC. If we do the right thing, and > there are no repercussions, I'd actually remove the warning.
Well, there is already <https://bugs.php.net/72784>.
> If you think > we might want to disallow this in the future, use an E_DEPRECATED, but a > warning seems pointless unless it's doing the wrong thing, or it's going to > start doing the wrong thing at some point.
+1
-- Christoph M. Becker

David Rodrigues

10 years ago
What is the sense to use min > max, in general? It should 'flip' arguments automatically in this case? 2016-08-08 18:36 GMT-03:00 Leigh <leight@gmail.com>:
> Hi all, > > There has been an unforeseen break with rand() when the minimum value is > greater than the maximum. > > Prior to 7.1 rand() would happily accept backwards parameters and return a > value, however in the 7.1 branch it now emits a warning and returns false. > > I've preemptively committed a fix to allow min > max and return a value as > in previous versions, but have kept the warning. > > Looking for some feedback/opinions on whether anyone else thinks this > should be fixed differently (or not at all). > > N.B. this also changes the behaviour of mt_rand to now accept min > max
-- David Rodrigues

Tom Worster

10 years ago
On 8/8/16 5:36 PM, Leigh wrote:
> Hi all, > > There has been an unforeseen break with rand() when the minimum value is > greater than the maximum. > > Prior to 7.1 rand() would happily accept backwards parameters and return a > value, however in the 7.1 branch it now emits a warning and returns false. > > I've preemptively committed a fix to allow min > max and return a value as > in previous versions, but have kept the warning. > > Looking for some feedback/opinions on whether anyone else thinks this > should be fixed differently (or not at all). > > N.B. this also changes the behaviour of mt_rand to now accept min > max
Your fix seems fine for rand() but less so for mt_rand(). Applying this fix will break much less mt_rand()-using code than not applying it will break rand()-using code. From that point of view, applying it is the better choice. Otoh, it's like copy-pasting a weird old bug from rand() to mt_rand(). The plan was to make rand() alias mt_rand(). Now I'm not sure that's a smart plan. Tom

Leigh

10 years ago
On Wed, 10 Aug 2016 at 14:19 Tom Worster <fsb@thefsb.org> wrote:
> Your fix seems fine for rand() but less so for mt_rand(). > > Applying this fix will break much less mt_rand()-using code than not > applying it will break rand()-using code. From that point of view, > applying it is the better choice. > > Otoh, it's like copy-pasting a weird old bug from rand() to mt_rand(). > The plan was to make rand() alias mt_rand(). Now I'm not sure that's a > smart plan. > > Tom >
I've pushed a fix that keeps the old behaviour for both I've un-aliased rand from mt_rand, added a min > max check to rand, and then call the common code with parameters reversed if necessary.