Grokbase › Groups › PHP › php-internals › December 2011
FAQ
Hi!

I see that test for bug 52062 (marked as fixed by this commit:
http://svn.php.net/viewvc/?view=revision&revision=320481) now fails
on my 32-bit system. Looking at the patch and the test, it can not
actually succeed, as the test expects this:
int(100000000000)
which is not possible on 32-bit system, and the code actually compares
the result to LONG_MAX which is 32-bit and returns false when it's
bigger that that. So I'd like to know what was the intent there:

1. Return false on 32-bit and the test should be fixed to account for
32-bit and 64-bit?
2. Return float on 32-bit?
3. Do something else?

Please advise.
--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227

Search Discussions

  • Derick Rethans at Dec 26, 2011 at 8:40 am ⇧

    Stas Malyshev wrote:

    Hi!

    I see that test for bug 52062 (marked as fixed by this commit:
    http://svn.php.net/viewvc/?view=revision&revision=320481) now
    fails
    on my 32-bit system. Looking at the patch and the test, it can not
    actually succeed, as the test expects this:
    int(100000000000)
    which is not possible on 32-bit system, and the code actually compares

    the result to LONG_MAX which is 32-bit and returns false when it's
    bigger that that. So I'd like to know what was the intent there:

    1. Return false on 32-bit and the test should be fixed to account for
    32-bit and 64-bit?
    2. Return float on 32-bit?
    3. Do something else?

    Please advise.
    --
    Stanislav Malyshev, Software Architect
    SugarCRM: http://www.sugarcrm.com/
    (408)454-6900 ext. 227

    --
    PHP Internals - PHP Runtime Development Mailing List
    To unsubscribe, visit: http://www.php.net/unsub.php
    I'll have a look when I get back home. Is there anything else you want me to look at?

    cheers,
    Derick
  • Antony Dovgal at Dec 26, 2011 at 8:56 am ⇧

    On 12/26/2011 12:40 PM, Derick Rethans wrote:
    I'll have a look when I get back home. Is there anything else you want me to look at?
    Yes!
    https://bugs.php.net/bug.php?id=53437 =)

    --
    Wbr,
    Antony Dovgal
    ---
    http://pinba.org - realtime profiling for PHP
  • Derick Rethans at Dec 27, 2011 at 12:27 pm ⇧

    On Mon, 26 Dec 2011, Antony Dovgal wrote:
    On 12/26/2011 12:40 PM, Derick Rethans wrote:

    I'll have a look when I get back home. Is there anything else you
    want me to look at?
    Yes!
    https://bugs.php.net/bug.php?id=53437 =)
    I've just had a look at the patch, and it seems to encode things in
    weird stuff for DatePeriod (packed ints, base64)?!. It also seems to
    re-implement the DateInterval serialisation and the patch doesn't
    cleanly apply.

    Gustavo, could you try find me on IRC?

    cheers,
    Derick


    --
    http://derickrethans.nl | http://xdebug.org
    Like Xdebug? Consider a donation: http://xdebug.org/donate.php
    twitter: @derickr and @xdebug
  • Gustavo Lopes at Dec 27, 2011 at 6:12 pm ⇧
    Hi
    On Tue, 27 Dec 2011 13:26:59 +0100, Derick Rethans wrote:
    On Mon, 26 Dec 2011, Antony Dovgal wrote:
    On 12/26/2011 12:40 PM, Derick Rethans wrote:

    I'll have a look when I get back home. Is there anything else you
    want me to look at?
    Yes!
    https://bugs.php.net/bug.php?id=53437 =)
    I've just had a look at the patch, and it seems to encode things in
    weird stuff for DatePeriod (packed ints, base64)?!.
    Not for DatePeriod, for DateInterval. The problem is that the DateInterval
    object uses 64-bit integers in its structure, and you can't serialize them
    as normal PHP integers if you want to have no data corruption in
    architectures with 32-bit longs like Windows. You seemed to have ignored
    that difficulty when you later implemented DateInterval serialization. My
    implementation also does state validation on DateInterval::__wakeup.
    It also seems to re-implement the DateInterval serialisation and the
    patch doesn't cleanly apply.
    Well, the patch is against r305891 (HEAD at the time), DateInterval
    serialization was implemented in r320481.

    In any case, if you want to use only the DatePeriod part (which provides
    serialization and properties), it should work independently from the
    DateInterval portion.

    --
    Gustavo Lopes
  • Stas Malyshev at Dec 26, 2011 at 9:08 am ⇧
    Hi!
    I'll have a look when I get back home. Thanks!
    Is there anything else you want me to look at?
    Yes, it would be nice if you could check out XFAILs, especially the one
    that crashes (53437) but others too.

    Thanks in advance,
    --
    Stanislav Malyshev, Software Architect
    SugarCRM: http://www.sugarcrm.com/
    (408)454-6900 ext. 227
  • Derick Rethans at Dec 27, 2011 at 4:00 pm ⇧

    On Sun, 25 Dec 2011, Stas Malyshev wrote:

    I see that test for bug 52062 (marked as fixed by this commit:
    http://svn.php.net/viewvc/?view=revision&revision=320481) now fails on my
    32-bit system. Looking at the patch and the test, it can not actually succeed,
    as the test expects this:
    int(100000000000)
    which is not possible on 32-bit system, and the code actually compares the
    result to LONG_MAX which is 32-bit and returns false when it's bigger that
    that. So I'd like to know what was the intent there:

    1. Return false on 32-bit and the test should be fixed to account for 32-bit
    and 64-bit?
    2. Return float on 32-bit?
    3. Do something else?

    Please advise.
    There are several issues here:

    1. The test was wrong, the result for var_dump($d->getTimestamp());
    should indeed have been bool: false; I've a patch for this.
    2. DateInterval::format() didn't handle large ints succesfully; I
    have a patch for this too.
    3. $d->setTimestamp(100000000000) is not ever going to work as
    zend_parse_parameters simply gives me long: 1215752192. This I find
    really strange actually. Any clue?

    cheers,
    Derick

    --
    http://derickrethans.nl | http://xdebug.org
    Like Xdebug? Consider a donation: http://xdebug.org/donate.php
    twitter: @derickr and @xdebug
  • Stas Malyshev at Dec 28, 2011 at 1:57 am ⇧
    Hi!
    3. $d->setTimestamp(100000000000) is not ever going to work as
    zend_parse_parameters simply gives me long: 1215752192. This I find
    really strange actually. Any clue?
    That's probably float to long conversion, since 100000000000 is float on
    32-bit.

    --
    Stanislav Malyshev, Software Architect
    SugarCRM: http://www.sugarcrm.com/
    (408)454-6900 ext. 227
  • Derick Rethans at Jan 4, 2012 at 9:01 pm ⇧

    On Tue, 27 Dec 2011, Stas Malyshev wrote:

    3. $d->setTimestamp(100000000000) is not ever going to work as
    zend_parse_parameters simply gives me long: 1215752192. This I find
    really strange actually. Any clue?
    That's probably float to long conversion, since 100000000000 is float on
    32-bit.
    Well, it isn't. Set a breakpoint on date_timestamp_set() and see what it
    does. It's really quite weird with what zend_parse_parameters does.

    Derick

    --
    http://derickrethans.nl | http://xdebug.org
    Like Xdebug? Consider a donation: http://xdebug.org/donate.php
    twitter: @derickr and @xdebug

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupphp-internals @
categoriesphp
postedDec 26, '11 at 12:33a
activeJan 4, '12 at 9:01p
posts9
users4
websitephp.net

4 users in discussion

Derick Rethans: 4 posts Stas Malyshev: 3 posts Gustavo Lopes: 1 post Antony Dovgal: 1 post

Content

People

Support

Translate

site design / logo © 2023 Grokbase