# zend_call_method() - support for up to 4 parameters - Thread: https://externals.io/message/43086 - Original archive: https://news-web.php.net/php.internals/43086 - Started by Nathan Nobbe on 2009-02-18T04:08:39+00:00 - Messages: 9 - 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 --- ## #43086 — Nathan Nobbe — 2009-02-18T04:08:39+00:00 URL: https://externals.io/message/43086#43086 hi, recently, working on an extension, i wanted to call a method w/ 3 params, and as you know, zend_call_method only supports 2 parameters at most. i came across this thread in the archives, http://marc.info/?l=php-internals&m=120179690310419&w=2 so i tossed together a quick patch w/ no emalloc or va_list against the latest 5.3 snapshot. what do you think? -nathan --- zend_interfaces.c 2009-02-17 20:50:35.000000000 -0700 +++ zend_interfaces.ORIG.c 2009-02-17 20:24:47.000000000 -0700 @@ -31,7 +31,7 @@ /* {{{ zend_call_method Only returns the returned zval if retval_ptr != `NULL` */ -ZEND_API zval* zend_call_method(zval **object_pp, zend_class_entry *obj_ce, zend_function **fn_proxy, char *function_name, int function_name_len, zval **retval_ptr_ptr, int param_count, zval* arg1, zval* arg2, zval* arg3, zval* arg4 TSRMLS_DC) +ZEND_API zval* zend_call_method(zval **object_pp, zend_class_entry *obj_ce, zend_function **fn_proxy, char *function_name, int function_name_len, zval **retval_ptr_ptr, int param_count, zval* arg1, zval* arg2 TSRMLS_DC) { int result; zend_fcall_info fci; @@ -39,12 +39,10 @@ zval *retval; HashTable *function_table; - zval **params[4]; + zval **params[2]; params[0] = &arg1; params[1] = &arg2; - params[2] = &arg3; - params[3] = &arg4; fci.size = sizeof(fci); /*fci.function_table = NULL; will be read form zend_class_entry of object if needed */ --- zend_interfaces.h 2009-02-17 20:51:22.000000000 -0700 +++ zend_interfaces.ORIG.h 2009-02-17 20:24:36.000000000 -0700 @@ -38,22 +38,16 @@ zval *value; } zend_user_iterator; -ZEND_API zval* zend_call_method(zval **object_pp, zend_class_entry *obj_ce, zend_function **fn_proxy, char *function_name, int function_name_len, zval **retval_ptr_ptr, int param_count, zval* arg1, zval* arg2, zval* arg3, zval* arg4 TSRMLS_DC); +ZEND_API zval* zend_call_method(zval **object_pp, zend_class_entry *obj_ce, zend_function **fn_proxy, char *function_name, int function_name_len, zval **retval_ptr_ptr, int param_count, zval* arg1, zval* arg2 TSRMLS_DC); #define zend_call_method_with_0_params(obj, obj_ce, fn_proxy, function_name, retval) \ - zend_call_method(obj, obj_ce, fn_proxy, function_name, sizeof(function_name)-1, retval, 0, NULL, NULL, NULL, `NULL` TSRMLS_CC) + zend_call_method(obj, obj_ce, fn_proxy, function_name, sizeof(function_name)-1, retval, 0, NULL, `NULL` TSRMLS_CC) #define zend_call_method_with_1_params(obj, obj_ce, fn_proxy, function_name, retval, arg1) \ - zend_call_method(obj, obj_ce, fn_proxy, function_name, sizeof(function_name)-1, retval, 1, arg1, NULL, NULL, `NULL` TSRMLS_CC) + zend_call_method(obj, obj_ce, fn_proxy, function_name, sizeof(function_name)-1, retval, 1, arg1, `NULL` TSRMLS_CC) #define zend_call_method_with_2_params(obj, obj_ce, fn_proxy, function_name, retval, arg1, arg2) \ - zend_call_method(obj, obj_ce, fn_proxy, function_name, sizeof(function_name)-1, retval, 2, arg1, arg2, NULL, `NULL` TSRMLS_CC) - -#define zend_call_method_with_3_params(obj, obj_ce, fn_proxy, function_name, retval, arg1, arg2, arg3) \ - zend_call_method(obj, obj_ce, fn_proxy, function_name, sizeof(function_name)-1, retval, 3, arg1, arg2, arg3, `NULL` TSRMLS_CC) - -#define zend_call_method_with_4_params(obj, obj_ce, fn_proxy, function_name, retval, arg1, arg2, arg3, arg4) \ - zend_call_method(obj, obj_ce, fn_proxy, function_name, sizeof(function_name)-1, retval, 4, arg1, arg2, arg3, arg4 TSRMLS_CC) + zend_call_method(obj, obj_ce, fn_proxy, function_name, sizeof(function_name)-1, retval, 2, arg1, arg2 TSRMLS_CC) ZEND_API void zend_user_it_rewind(zend_object_iterator *_iter TSRMLS_DC); ZEND_API int zend_user_it_valid(zend_object_iterator *_iter TSRMLS_DC); --- ## #43087 — Antony Dovgal — 2009-02-18T08:34:51+00:00 URL: https://externals.io/message/43086#43087 · In reply to: #43086 > hi, > > recently, working on an extension, i wanted to call a method w/ 3 params, > and as you know, zend_call_method only supports 2 parameters at most. i > came across this thread in the archives, > > http://marc.info/?l=php-internals&m=120179690310419&w=2 > > so i tossed together a quick patch w/ no emalloc or va_list against the > latest 5.3 snapshot. > > what do you think? What happened to call_user_function() ? Why break the API and make extension maintainers use even more #if's ? -- Wbr, Antony Dovgal --- ## #43090 — johannes@php.net — 2009-02-18T13:16:44+00:00 URL: https://externals.io/message/43086#43090 · In reply to: #43087 > > […] [...] > What happened to call_user_function() ? > Why break the API and make extension maintainers use even more #if's ? The benefit of zend_call_method is that you can keep the function pointer and so the lookup has not to do the lookup every time it is called which is quite nice. But I don't think that a new limitation is any better: Tomorrow we have to change it again as somebody has a reason to use 5 parameters, so if it is changed it should be changed to take any number of arguments and no fixed limit.... (or add a new API for that) johannes --- ## #43093 — Nathan Nobbe — 2009-02-18T14:31:56+00:00 URL: https://externals.io/message/43086#43093 · In reply to: #43090 > But I don't think that a new limitation is any better: Tomorrow we have > to change it again as somebody has a reason to use 5 parameters, so if > it is changed it should be changed to take any number of arguments and > no fixed limit.... (or add a new API for that) that does seem ideal to me, but in the last thread, marcus was clear that an emalloc / va_list solution was unacceptable. that was last january.. has anything changed in that regard since? also, it sounded like he was saying a simple solution for up to 4 parameters would be alright. mainly i thought it would be nice to get it into the code for 5.3 before we'd have to wait for another major release in order to get something included. yes, perhaps the function would need updating again in the future, but since people are using macros rather than zend_call_method directly it wouldnt be too bad right? -nathan --- ## #43100 — Marcus Boerger — 2009-02-18T23:14:47+00:00 URL: https://externals.io/message/43086#43100 · In reply to: #43093 Hello Nathan, Wednesday, February 18, 2009, 3:31:56 PM, you wrote: > > > […] > that does seem ideal to me, but in the last thread, marcus was clear that an > emalloc / va_list solution was unacceptable. that was last january.. has > anything changed in that regard since? No. > also, it sounded like he was saying a simple solution for up to 4 parameters > would be alright. mainly i thought it would be nice to get it into the code > for 5.3 before we'd have to wait for another major release in order to get > something included. Stuff like this can imo always be added and whether you do #if `PHP_VERSION_ID` >= 50300 or #if `PHP_VERSION_ID` >= 50301 imo doesn't matter much. > yes, perhaps the function would need updating again in the future, but since > people are using macros rather than zend_call_method directly it wouldnt be > too bad right? No. I think if you have a reasonable need for 4 paramters, then that's good enough. On the other hand you shouldn't argue with me about 4 params :-) > -nathan Best regards, Marcus --- ## #43110 — johannes@php.net — 2009-02-19T10:57:09+00:00 URL: https://externals.io/message/43086#43110 · In reply to: #43100 > Stuff like this can imo always be added and whether you do > #if `PHP_VERSION_ID` >= 50300 > or > #if `PHP_VERSION_ID` >= 50301 > imo doesn't matter much. It does. Up till now we always tried to keep binary compatibility for "bug fix" releases (z in x.y.z) We have a few vendors of binary extensions (not only Zend ...) which rely on the fact that they can provide one extension build for the whole series - at least as long as their extension aren't reaching too deep into the engine. Oh, and I think this also affects our own Windows builds of PECL stuff... Yes, our development process and release structure might need some updates, but version numbering might be the result from changes there, not the beginning. johannes ps. I'm aware of the fact that we added some specific APIs in special cases in bug fix releases before, but there's a difference between adding APIs and breaking the ABI of existing, used, stuff. --- ## #43111 — Pierre Joye — 2009-02-19T11:22:41+00:00 URL: https://externals.io/message/43086#43111 · In reply to: #43110 hi, > ps. I'm aware of the fact that we added some specific APIs in special > cases in bug fix releases before, but there's a difference between > adding APIs and breaking the ABI of existing, used, stuff. Exactly. To summarize: x.y.z to x.y.z+1: ABI and API must be 100% compatible x.y.z to x.y+1.z: ABI can be broken (need a recompilation), API must be 100% compatible x.y.z to x+1.y.z: party time ;) Cheers, -- Pierre http://blog.thepimp.net | http://www.libgd.org --- ## #43116 — Nathan Nobbe — 2009-02-19T14:41:32+00:00 URL: https://externals.io/message/43086#43116 · In reply to: #43110 did you guys see the other patch i posted yesterday? i re-read Johannes' first reply yesterday and saw the bit about a second api for a zend_call_method with a variable number of parameters. so a made a new function then re-implemented zend_call_method based on that. heres the patch (easier to read on pastebin). http://pastebin.com/m4fd58822 im sure it could be tweaked, but i think the main point is using an array rather than va_list to support the varialbe arg list. is this any better than the first one ? thanks, -nathan --- ## #43144 — Pierre Joye — 2009-02-21T13:46:01+00:00 URL: https://externals.io/message/43086#43144 > Hello Pierre, > > Thursday, February 19, 2009, 12:22:41 PM, you wrote: > > > […] > > > […] > > > […] > > > […] > > > […] > > That's completely untrue. In your mind maybe, but if we do not follow these simple rules then we have a serious problem. > What we ensure is that function f introduced in > x.y.z stays in all verisons x.y.z+n and keeps its exact semantics. We have been wrong in the past and changed API between minor version, it was a major mistake. In the meantime, we have tried to do not do it again. x.y.z and x.y.y+1 has not only have to keep the same sematic, behaviors but also the same signatures as well as a binary compatibility. One bad example was a change we made in 5.2.x making it incompatible with previous versions, there was no version id chante and it was a pain to change the extensions to be compatible with the full 5.2.x serie. The worst is that I can't even figure out when it happens without looking at the code (maybe 5.2.5? :) > What we > did not ensure ever, is that version x.y.z+1 does not come with a function > g that did not exist in x.y.z. That's no problem, as long as the x.y.z+1 API is 100% compatible with x.y.y. New functions do not break existing functions signature or behaviors, obviously. > Same obviously applies to consts and yes > macros, which have nothing to do abi or api. And actually abi has nothing > to do with the whole discussion. The ABI influences how a binary gets > layout, ABI has to do with the discussions as a given binary has to work across the same minor versions. It causes the same problem but with different origins. > and in that way determines how a user of said binary can access the > API elements (consts, function, classes,...). This obviously affects DLLs > on windows and shared objects on *nix. Either way, adding more macros and > new functions is fine, as lone as we do not change any existing function > protocol. Exactly what I said. New functions do not break the API and it is possible to modify a struct without breaking existing binary compatibility. > The new patch is clean enough to me and doesn't change the API. Whether the > ABI changes depends on the compilers and linkers. My comment was a general comment on what should be the policy, not specifically about this patch :) All in all it seems that you are saying the same than I did, only with different wording :) Cheers, -- Pierre http://blog.thepimp.net | http://www.libgd.org