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:
- Return false on 32-bit and the test should be fixed to account for
32-bit and 64-bit? - Return float on 32-bit?
- Do something else?
Please advise.
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227
Stas Malyshev smalyshev@sugarcrm.com 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 comparesthe 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:
- Return false on 32-bit and the test should be fixed to account for
32-bit and 64-bit?- Return float on 32-bit?
- Do something else?
Please advise.
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227--
I'll have a look when I get back home. Is there anything else you want me to look at?
cheers,
Derick
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
I'll have a look when I get back home. Is there anything else you
want me to look at?
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
Hi
I'll have a look when I get back home. Is there anything else you
want me to look at?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
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
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:
- Return false on 32-bit and the test should be fixed to account for 32-bit
and 64-bit?- Return float on 32-bit?
- Do something else?
Please advise.
There are several issues here:
- The test was wrong, the result for var_dump($d->getTimestamp());
should indeed have been bool: false; I've a patch for this. - DateInterval::format() didn't handle large ints succesfully; I
have a patch for this too. - $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
Hi!
- $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
- $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