# cleaning up the functions - any volunteers? - Thread: https://externals.io/message/38436 - Original archive: https://news-web.php.net/php.internals/38436 - Started by Stanislav Malyshev on 2008-06-19T23:06:00+00:00 - Messages: 34 - Page 1 of 2: messages 1 to 25 (25 per page, add `?page=N` to the URL) - Next page: https://externals.io/message/38436.md?page=2 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #38436 — Stanislav Malyshev — 2008-06-19T23:06:00+00:00 URL: https://externals.io/message/38436#38436 Hi! While we nearing the release of 5.3 (hopefully?), there are many functions in the PHP code which still use old parameter parsing API (zend_get_parameters_ex) instead of the new one (zend_parse_parameters). I have cleaned up Zend engine functions, converting them to the new API, but there are about 1000 instances throughout PHP code (especially ext/standard) which still use the old way. This way is less correct, inconsistent with the new code, gives worse error messages, more prone to various bugs, etc. All new extensions and functions use the new way, and the only reason for keeping the old way in the code seems to be that nobody cleaned it up. Somebody has to bring the old ones into sync, and I don't think I personally will have time to do it in near timeframe. So if anybody could step up to that - by doing at least part of the work - that'd be great. The work is pretty simple, taking something like: zval **data; if (ZEND_NUM_ARGS() != 1 || zend_get_parameters_ex(1, &data) == FAILURE) { WRONG_PARAM_COUNT; } convert_to_string_ex(data); and move it into: char *str; int str_len; if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s", &str, &str_len) == FAILURE) { return; } and of course convert use of data zval into use of str. This needs to be done carefully as not to break things on some more tricky functions which may get arguments of multiple types, etc. Also, some tests which check error messages may need to be fixed since new API gives more detailed messages. zend_parse_parameters documented here: http://www.php.net/manual/en/internals2.ze1.zendapi.php though some of the newer parameters aren't (C, h, f, Z) so one may need to look at the code at zend_API.c. P.S. if some genius would invent a script to automate that - it'd be awesome, but I don't have very high hopes on that. :) -- Stanislav Malyshev, Zend Software Architect stas@zend.com http://www.zend.com/ (408)253-8829 MSN: stas@zend.com --- ## #38437 — Felipe Pena — 2008-06-19T23:23:12+00:00 URL: https://externals.io/message/38436#38437 · In reply to: #38436 Hello Em Qui, 2008-06-19 às 16:06 -0700, Stanislav Malyshev escreveu: > Hi! > > While we nearing the release of 5.3 (hopefully?), there are many > functions in the PHP code which still use old parameter parsing API > (zend_get_parameters_ex) instead of the new one (zend_parse_parameters). > Yeah, I have noticed... That's also in my TODO. -- Regards, Felipe Pena. --- ## #38441 — Philip Olson — 2008-06-20T01:37:21+00:00 URL: https://externals.io/message/38436#38441 · In reply to: #38437 > > […] > > Yeah, I have noticed... That's also in my TODO. Where is the Felipe fan club signup sheet? Regards, Philip --- ## #38470 — David Coallier — 2008-06-20T15:18:50+00:00 URL: https://externals.io/message/38436#38470 · In reply to: #38436 I'll take a few 2008/6/20 Stanislav Malyshev : > Hi! > > While we nearing the release of 5.3 (hopefully?), there are many functions > in the PHP code which still use old parameter parsing API > (zend_get_parameters_ex) instead of the new one (zend_parse_parameters). > > I have cleaned up Zend engine functions, converting them to the new API, > but there are about 1000 instances throughout PHP code (especially > ext/standard) which still use the old way. This way is less correct, > inconsistent with the new code, gives worse error messages, more prone to > various bugs, etc. All new extensions and functions use the new way, and the > only reason for keeping the old way in the code seems to be that nobody > cleaned it up. Somebody has to bring the old ones into sync, and I don't > think I personally will have time to do it in near timeframe. So if anybody > could step up to that - by doing at least part of the work - that'd be > great. The work is pretty simple, taking something like: > > zval **data; > > if (ZEND_NUM_ARGS() != 1 || zend_get_parameters_ex(1, &data) == FAILURE) { > WRONG_PARAM_COUNT; > } > convert_to_string_ex(data); > > and move it into: > > char *str; > int str_len; > > if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s", &str, &str_len) > == FAILURE) { > return; > } > > and of course convert use of data zval into use of str. > > This needs to be done carefully as not to break things on some more tricky > functions which may get arguments of multiple types, etc. Also, some tests > which check error messages may need to be fixed since new API gives more > detailed messages. > zend_parse_parameters documented here: > http://www.php.net/manual/en/internals2.ze1.zendapi.php > though some of the newer parameters aren't (C, h, f, Z) so one may need to > look at the code at zend_API.c. > > P.S. if some genius would invent a script to automate that - it'd be > awesome, but I don't have very high hopes on that. :) > -- > Stanislav Malyshev, Zend Software Architect > stas@zend.com http://www.zend.com/ > (408)253-8829 MSN: stas@zend.com > > -- > > > > -- Slan, David --- ## #38471 — Olivier Hill — 2008-06-20T15:35:41+00:00 URL: https://externals.io/message/38436#38471 · In reply to: #38470 Want some help D? :) Olivier > I'll take a few --- ## #38482 — David Coallier — 2008-06-20T16:45:06+00:00 URL: https://externals.io/message/38436#38482 · In reply to: #38471 2008/6/20 Olivier Hill : > Want some help D? :) > > Olivier > > > > […] > Yep, after speaking with felipe, we (you and I) have standard/ -- Slan, David --- ## #38485 — Olivier Hill — 2008-06-20T17:49:15+00:00 URL: https://externals.io/message/38436#38485 · In reply to: #38482 Stan, Can you confirm you're not already working on this? Tony told me on IRC you were doing something. I was just about to commit ext/standard/var.c & tests. David and I are going to go through ext/standard/* Thanks Olivier > > Yep, after speaking with felipe, we (you and I) have standard/ --- ## #38486 — Stanislav Malyshev — 2008-06-20T17:55:39+00:00 URL: https://externals.io/message/38436#38486 · In reply to: #38485 Hi! > Can you confirm you're not already working on this? Tony told me on > IRC you were doing something. I was just about to commit Yes, I'm not working on ext/standard. I was doing Zend engine, but that is done. -- Stanislav Malyshev, Zend Software Architect stas@zend.com http://www.zend.com/ (408)253-8829 MSN: stas@zend.com --- ## #38502 — Alexey Zakhlestin — 2008-06-21T21:24:50+00:00 URL: https://externals.io/message/38436#38502 · In reply to: #38436 I did curl for 5.3 > Hi! > > While we nearing the release of 5.3 (hopefully?), there are many functions > in the PHP code which still use old parameter parsing API > (zend_get_parameters_ex) instead of the new one (zend_parse_parameters). > > I have cleaned up Zend engine functions, converting them to the new API, > but there are about 1000 instances throughout PHP code (especially > ext/standard) which still use the old way. This way is less correct, > inconsistent with the new code, gives worse error messages, more prone to > various bugs, etc. All new extensions and functions use the new way, and the > only reason for keeping the old way in the code seems to be that nobody > cleaned it up. Somebody has to bring the old ones into sync, and I don't > think I personally will have time to do it in near timeframe. So if anybody > could step up to that - by doing at least part of the work - that'd be > great. The work is pretty simple, taking something like: > > zval **data; > > if (ZEND_NUM_ARGS() != 1 || zend_get_parameters_ex(1, &data) == FAILURE) { > WRONG_PARAM_COUNT; > } > convert_to_string_ex(data); > > and move it into: > > char *str; > int str_len; > > if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s", > &str, &str_len) == FAILURE) { > return; > } > > and of course convert use of data zval into use of str. > > This needs to be done carefully as not to break things on some more tricky > functions which may get arguments of multiple types, etc. Also, some tests > which check error messages may need to be fixed since new API gives more > detailed messages. > zend_parse_parameters documented here: > http://www.php.net/manual/en/internals2.ze1.zendapi.php > though some of the newer parameters aren't (C, h, f, Z) so one may need to > look at the code at zend_API.c. > > P.S. if some genius would invent a script to automate that - it'd be > awesome, but I don't have very high hopes on that. :) > -- > Stanislav Malyshev, Zend Software Architect > stas@zend.com http://www.zend.com/ > (408)253-8829 MSN: stas@zend.com > > -- > > > > -- Alexey Zakhlestin http://blog.milkfarmsoft.com/ --- ## #38503 — David Coallier — 2008-06-21T21:28:09+00:00 URL: https://externals.io/message/38436#38503 · In reply to: #38502 2008/6/21 Alexey Zakhlestin : > I did curl for 5.3 > Grand! did you commit it already? -- Slan, David --- ## #38504 — Alexey Zakhlestin — 2008-06-21T21:32:27+00:00 URL: https://externals.io/message/38436#38504 · In reply to: #38503 > 2008/6/21 Alexey Zakhlestin : > > > […] I don't have karma. did attachment come to the list? anyway, here is the file: http://filez.indeyets.pp.ru/curl.diff -- Alexey Zakhlestin http://blog.milkfarmsoft.com/ --- ## #38505 — Rasmus Lerdorf — 2008-06-21T21:35:48+00:00 URL: https://externals.io/message/38436#38505 · In reply to: #38504 Alexey Zakhlestin wrote: > > > […] > > I don't have karma. You do now. --- ## #38506 — David Coallier — 2008-06-21T21:40:56+00:00 URL: https://externals.io/message/38436#38506 · In reply to: #38505 2008/6/21 Rasmus Lerdorf : > Alexey Zakhlestin wrote: > > […] > > You do now. Cheers Mr. L -- Slan, David --- ## #38507 — Alexey Zakhlestin — 2008-06-21T22:01:12+00:00 URL: https://externals.io/message/38436#38507 · In reply to: #38505 > You do now. thanks. checked in -- Alexey Zakhlestin http://blog.milkfarmsoft.com/ --- ## #38509 — Lukas Kahwe Smith — 2008-06-22T14:14:55+00:00 URL: https://externals.io/message/38436#38509 · In reply to: #38436 > While we nearing the release of 5.3 (hopefully?), there are many > functions in the PHP code which still use old parameter parsing API > (zend_get_parameters_ex) instead of the new one > (zend_parse_parameters). is this in any way related to the parameter parsing API change that caused the BC break in `array_merge()` back in PHP 5.0? If so then I would appreciate extra care to be taken to minimize these kinds of BC breaks and at least make sure that these breaks are well documented. regards, Lukas Kahwe Smith mls@pooteeweet.org --- ## #38510 — David Coallier — 2008-06-22T15:09:21+00:00 URL: https://externals.io/message/38436#38510 · In reply to: #38509 2008/6/22 Lukas Kahwe Smith : > > > > > […] > > > is this in any way related to the parameter parsing API change that caused > the BC break in `array_merge()` back in PHP 5.0? If so then I would appreciate > extra care to be taken to minimize these kinds of BC breaks and at least > make sure that these breaks are well documented. Ok, with the tests being ran we are minimizing the chances of bc break but I agree and extra care should be applied. We'll make sure this care is applied. > > regards, > Lukas Kahwe Smith > mls@pooteeweet.org > > > > > -- > > > > -- Slan, David --- ## #38511 — David Coallier — 2008-06-22T15:12:17+00:00 URL: https://externals.io/message/38436#38511 · In reply to: #38510 > > Ok, with the tests being ran we are minimizing the chances of bc break > but I agree and extra care should be applied. We'll make sure this > care is applied. > > And sorry about the email before, I forgot to remove the junk. :) -- Slan, David --- ## #38512 — Pierre Joye — 2008-06-22T15:18:42+00:00 URL: https://externals.io/message/38436#38512 · In reply to: #38510 Hi David, > > […] > > Ok, with the tests being ran we are minimizing the chances of bc break > but I agree and extra care should be applied. We'll make sure this > care is applied. http://wiki.php.net/doc/scratchpad/upgrade/53 -- Pierre http://blog.thepimp.net | http://www.libgd.org --- ## #38513 — Olivier Hill — 2008-06-22T15:40:35+00:00 URL: https://externals.io/message/38436#38513 · In reply to: #38512 > > […] > > http://wiki.php.net/doc/scratchpad/upgrade/53 I will be documenting BC breaks on that page after committing. Basically, some functions like `trim()` would not behave the same way when passing things like resources, arrays or objects without a __toString() method. The old ways didn't make any sense most of the time anyway, as you don't want to trim a resource and get a string back containing: "Resource" Ex: $ar = array(1, 2); Before: var_dump(trim($ar)) -> string(5) "Array" with an `E_NOTICE` Now: var_dump(trim($ar)) -> `NULL` with an `E_WARNING` Tests are going to be changed to reflect that also (that's mostly the longest part) Regards, Olivier --- ## #38520 — Stanislav Malyshev — 2008-06-22T21:21:16+00:00 URL: https://externals.io/message/38436#38520 · In reply to: #38509 Hi! > is this in any way related to the parameter parsing API change that > caused the BC break in `array_merge()` back in PHP 5.0? If so then I would I don't really remember about array_merge in detail, but this change shouldn't (I know, famous last words :) create any BC break unless you rely on weird stuff like array-to-string implicit conversion (which you really really shouldn't touch with a ten foot pole anyway). Unfortunately, here we in general can do only as good as unit tests do, so if anyone knows places where it might hurt, you may want to pay extra attention there. -- Stanislav Malyshev, Zend Software Architect stas@zend.com http://www.zend.com/ (408)253-8829 MSN: stas@zend.com --- ## #38521 — Lukas Kahwe Smith — 2008-06-22T21:32:39+00:00 URL: https://externals.io/message/38436#38521 · In reply to: #38520 > Hi! > > > […] > > I don't really remember about array_merge in detail, but this change > shouldn't (I know, famous last words :) create any BC break unless > you rely on weird stuff like array-to-string implicit conversion > (which you really really shouldn't touch with a ten foot pole anyway). The break in `array_merge()` was that previously it would cast the parameters to an array. As such a popular use case before the parameter parsing change was the following: $foo = null; $bar = array_merge($foo, $array); Since 5.0 this would lead to $bar being false and not $array. While it could be argued that the user should do an explicit cast himself, it was still a clear BC issue that caused a lot of issues for developers. regards, Lukas Kahwe Smith mls@pooteeweet.org --- ## #38523 — Stanislav Malyshev — 2008-06-22T21:51:45+00:00 URL: https://externals.io/message/38436#38523 · In reply to: #38521 Hi! > Since 5.0 this would lead to $bar being false and not $array. While it > could be argued that the user should do an explicit cast himself, it was > still a clear BC issue that caused a lot of issues for developers. Generally I think it would be nice if all functions behaved uniformly (i.e. zend_parse_parameters has a standard, all use it). But we could create special cases by accepting things as "z" and doing whatever we want, I guess. -- Stanislav Malyshev, Zend Software Architect stas@zend.com http://www.zend.com/ (408)253-8829 MSN: stas@zend.com --- ## #38530 — Timm Friebe — 2008-06-23T07:55:03+00:00 URL: https://externals.io/message/38436#38530 · In reply to: #38436 Hi, > While we nearing the release of 5.3 (hopefully?), there are many > functions in the PHP code which still use old parameter parsing API > (zend_get_parameters_ex) instead of the new one (zend_parse_parameters). I started taking care of ext/sybase_ct. - Timm --- ## #38621 — crrodriguez@suse.de — 2008-06-26T00:12:28+00:00 URL: https://externals.io/message/38436#38621 · In reply to: #38436 Stanislav Malyshev escribió: > Hi! > > While we nearing the release of 5.3 (hopefully?), there are many > functions in the PHP code which still use old parameter parsing API > (zend_get_parameters_ex) instead of the new one (zend_parse_parameters). > > P.S. if some genius would invent a script to automate that - it'd be > awesome, but I don't have very high hopes on that. :) The attached patch may also help both core and PECL extensions, emiting a deprecation compile warning when those functions are used. ps: you have to add the attribute to all deprecated functions ;-) -- Cristian Rodríguez R. Platform/OpenSUSE - Core Services SUSE LINUX Products GmbH Research & Development http://www.opensuse.org/ --- ## #38622 — Stanislav Malyshev — 2008-06-26T00:51:29+00:00 URL: https://externals.io/message/38436#38622 · In reply to: #38621 Hi! > The attached patch may also help both core and PECL extensions, emiting > a deprecation compile warning when those functions are used. I think it's a good idea. Any objections from anybody? -- Stanislav Malyshev, Zend Software Architect stas@zend.com http://www.zend.com/ (408)253-8829 MSN: stas@zend.com --- Next page: https://externals.io/message/38436.md?page=2