zend_call_method() - support for up to 4 parameters

16 years ago by Nathan Nobbe — view source — reply

unread

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);

16 years ago by Antony Dovgal — view source — reply

unread

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

16 years ago by johannes@php.net — view source — reply

unread

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,
[...]
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

16 years ago by Nathan Nobbe — view source — reply

unread

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

16 years ago by Marcus Boerger — view source — reply

unread

Hello Nathan,

Wednesday, February 18, 2009, 3:31:56 PM, you wrote:

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?

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

16 years ago by johannes@php.net — view source — reply

unread

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.

16 years ago by Pierre Joye — view source — reply

unread

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

16 years ago by Nathan Nobbe — view source — reply

unread

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

16 years ago by Pierre Joye — view source — reply

unread

Hello Pierre,

Thursday, February 19, 2009, 12:22:41 PM, you wrote:

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 ;)

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