18 years ago by Pierre Joye — view source — reply

unread

Hi!

Hi all,

I made a patch for 5_3 where all functions in ext/standard/math.c use
the new parameter parsing. (As was done in HEAD)

I think it got lost between you and the list :)

If anyone think it good, let me know, and then i'll commit it.

Several tests break with the patch. Hence, i'll fix them also, if
agreed. :)

How do they fail? I worry a bit about BC here.

Thanks for your work!

Pierre
http://blog.thepimp.net | http://www.libgd.org

18 years ago by Felipe Pena — view source — reply

unread

Em Qua, 2008-02-13 às 01:31 +0100, Pierre Joye escreveu:

Hi!

Hi all,

I made a patch for 5_3 where all functions in ext/standard/math.c use
the new parameter parsing. (As was done in HEAD)

I think it got lost between you and the list :)

If anyone think it good, let me know, and then i'll commit it.

Several tests break with the patch. Hence, i'll fix them also, if
agreed. :)

How do they fail? I worry a bit about BC here.

There are two cases.

In most part of it:
003+ Warning: acos() expects exactly 1 parameter, 2 given ...
003- Warning: Wrong parameter count for acos() ...

And breaking BC:
010+
011+ Warning: acos() expects parameter 1 to be double, string given ...
012+ NULL
010- float(1.570796327)
014+
015+ Notice: A non well formed numeric value encountered ...

PS: The same behavior in HEAD.

http://felipe.ath.cx/diff/math.diff

Thanks for your work!

Thanks for your attention! :)

Regards,
Felipe Pena.

18 years ago by David Coallier — view source — reply

unread

Em Qua, 2008-02-13 às 01:31 +0100, Pierre Joye escreveu:

Hi!

Hi all,

I made a patch for 5_3 where all functions in ext/standard/math.c use
the new parameter parsing. (As was done in HEAD)

I think it got lost between you and the list :)

If anyone think it good, let me know, and then i'll commit it.

Several tests break with the patch. Hence, i'll fix them also, if
agreed. :)

How do they fail? I worry a bit about BC here.

There are two cases.

In most part of it:
003+ Warning: acos() expects exactly 1 parameter, 2 given ...
003- Warning: Wrong parameter count for acos() ...

And breaking BC:
010+
011+ Warning: acos() expects parameter 1 to be double, string given ...
012+ NULL > 010- float(1.570796327)
014+
015+ Notice: A non well formed numeric value encountered ...

PS: The same behavior in HEAD.

http://felipe.ath.cx/diff/math.diff

Thanks for your work!

Thanks for your attention! :)

Regards,
Felipe Pena.

--

As long as it's in 5_3 I believe there'll be no problems. I did the
same for a few string.c functions.

As long as the behavior itself doesn't change, I'd say good work :)

--
David Coallier,
Founder & Software Architect,
Agora Production (http://agoraproduction.com)
51.42.06.70.18

18 years ago by Antony Dovgal — view source — reply

unread

As long as it's in 5_3 I believe there'll be no problems. I did the
same for a few string.c functions.

As long as the behavior itself doesn't change, I'd say good work :)

Agree, it's ok in 5_3.
We need to convert all the functions to use new param parsing API anyway.

Some more tests would not hurt though =)

--
Wbr,
Antony Dovgal

18 years ago by Pierre Joye — view source — reply

unread

Em Qua, 2008-02-13 às 01:31 +0100, Pierre Joye escreveu:

Hi!

Hi all,

I made a patch for 5_3 where all functions in ext/standard/math.c use
the new parameter parsing. (As was done in HEAD)

I think it got lost between you and the list :)

If anyone think it good, let me know, and then i'll commit it.

Several tests break with the patch. Hence, i'll fix them also, if
agreed. :)

How do they fail? I worry a bit about BC here.

There are two cases.

In most part of it:
003+ Warning: acos() expects exactly 1 parameter, 2 given ...
003- Warning: Wrong parameter count for acos() ...

Ok, any branch.

And breaking BC:
010+
011+ Warning: acos() expects parameter 1 to be double, string given ...
012+ NULL
010- float(1.570796327)
014+
015+ Notice: A non well formed numeric value encountered ...

I tend to think to do not break such things in 5.x. But 5.3 sounds
like a good opportunity to add a notice about the bad inputs. I can't
test the patch right now but does it change the result or only raises
a notice?

Cheers,

Pierre
http://blog.thepimp.net | http://www.libgd.org

18 years ago by Felipe Pena — view source — reply

unread

Em Qua, 2008-02-13 às 09:47 +0100, Pierre Joye escreveu:

I can't test the patch right now but does it change the result or only raises
a notice?

It changes the result when a string is given, and issue notice when the
string starts with numeric.

var_dump(acos("nonsense"));
Warning: acos() expects parameter 1 to be double, string given ...
NULL

var_dump(acos("1000ABC"));
Notice: A non well formed numeric value encountered ...
float(NAN)

18 years ago by Pierre Joye — view source — reply

unread

Em Qua, 2008-02-13 às 09:47 +0100, Pierre Joye escreveu:

I can't test the patch right now but does it change the result or only raises
a notice?

It changes the result when a string is given, and issue notice when the
string starts with numeric.

var_dump(acos("nonsense"));
Warning: acos() expects parameter 1 to be double, string given ...
NULL

That's a BC but it seems to be sane to do it. It is amazing that the
current version returns acos(0) (obvious but still amazing :).

var_dump(acos("1000ABC"));
Notice: A non well formed numeric value encountered ...
float(NAN)

No BC here, as far as I can tell. It returns the same result as before.

Thanks for your great work! :)

Pierre
http://blog.thepimp.net | http://www.libgd.org

18 years ago by Jani Taskinen — view source — reply

unread

Em Qua, 2008-02-13 às 09:47 +0100, Pierre Joye escreveu:

I can't test the patch right now but does it change the result or only raises
a notice?

It changes the result when a string is given, and issue notice when the
string starts with numeric.

var_dump(acos("nonsense"));
Warning: acos() expects parameter 1 to be double, string given ...
NULL

var_dump(acos("1000ABC"));
Notice: A non well formed numeric value encountered ...
float(NAN)

php -n -derror_reporting=E_ALL -r 'var_dump(acos("ABC"));'

float(1.5707963267949)

php -n -derror_reporting=E_ALL -r 'var_dump(acos("100ABC"));'

float(NAN)

That's with PHP 5.2.5. I rather like the idea of these functions not
doing magical conversion like this.

--
Patches/Donations: http://pecl.php.net/~jani/

18 years ago by zoe.slattery@googlemail.com — view source — reply

unread

Felipe Pena wrote:

Hi all,

I made a patch for 5_3 where all functions in ext/standard/math.c use
the new parameter parsing. (As was done in HEAD)
If anyone think it good, let me know, and then i'll commit it.

Several tests break with the patch. Hence, i'll fix them also, if
agreed. :)

Felipe - Great work!! I'll wait for the patch to go in and then re-run
the tests that I'm still trying to debug to see if they pass.

Zoe

18 years ago by Marcus Boerger — view source — reply

unread

Hello Felipe,

nice work. The others already talked about the implications. And given
what I can only chime in. Since so far people think it is a ok for 5.3 I
wonder if we should not continue with moving towards the new parameter
parsing API. In the end the best would be to get rid of the old one. Anyway
thanks for the work.

marcus

Wednesday, February 13, 2008, 1:04:05 AM, you wrote:

Hi all,

I made a patch for 5_3 where all functions in ext/standard/math.c use
the new parameter parsing. (As was done in HEAD)
If anyone think it good, let me know, and then i'll commit it.

Several tests break with the patch. Hence, i'll fix them also, if
agreed. :)

--
Regards,
Felipe Pena.

Best regards,
Marcus

Math functions (new parameter parsing) View as Markdown

Thanks for your work!

Thanks for your attention! :)

Thanks for your attention! :)

Cheers,

Thanks for your great work! :)

php -n -derror_reporting=E_ALL -r 'var_dump(acos("ABC"));'

php -n -derror_reporting=E_ALL -r 'var_dump(acos("100ABC"));'