function registration patch

php.internals

Andrei Zmievski

23 years ago
Currently when registering functions, the unmodified function name is used as the key for the function table. This leads to problems when an extension class has a constructor that is not all lowercased. For example: function_entry php_gtk_button_functions[] = { PHP_FE("GtkButton", NULL), ... }; Then when using something like: ... parent::GtkButton(); ... It fails because it uses 'gtkbutton' as the key to search the function table. The attached patch fixes that by lowercasing the key (function name) when registering functions. I know that some may say, "just use lowercase class name for the constructor in the function entry list", but that messes with nice class/function names again. -Andrei http://www.gravitonic.com/ * It said 'Winmodem' on the box, but I still feel like I lost. *

Derick Rethans

23 years ago
On Tue, 18 Mar 2003, Andrei Zmievski wrote:
> Currently when registering functions, the unmodified function name is > used as the key for the function table. This leads to problems when an > extension class has a constructor that is not all lowercased. For > example: > > function_entry php_gtk_button_functions[] = { > PHP_FE("GtkButton", NULL), > ... > }; > > Then when using something like: > > ... > parent::GtkButton(); > ... > > It fails because it uses 'gtkbutton' as the key to search the function > table. The attached patch fixes that by lowercasing the key (function > name) when registering functions. > > I know that some may say, "just use lowercase class name for the > constructor in the function entry list", but that messes with nice > class/function names again.
Maybe we can finally fix the OO model and just make them case-sensitive? :) If that's not an option, I think the patch is OK. Derick
-- "my other box is your windows PC" ------------------------------------------------------------------------- Derick Rethans http://derickrethans.nl/ PHP Magazine - PHP Magazine for Professionals http://php-mag.net/ -------------------------------------------------------------------------

Andrei Zmievski

23 years ago
Grr. Totally slipped my mind that with this patch, the original function names will be lost when doing get_class_methods() and similar stuff. Any other suggestions? On Tue, 18 Mar 2003, Andrei Zmievski wrote:
> Currently when registering functions, the unmodified function name is > used as the key for the function table. This leads to problems when an > extension class has a constructor that is not all lowercased. For > example: > > function_entry php_gtk_button_functions[] = { > PHP_FE("GtkButton", NULL), > ... > }; > > Then when using something like: > > ... > parent::GtkButton(); > ... > > It fails because it uses 'gtkbutton' as the key to search the function > table. The attached patch fixes that by lowercasing the key (function > name) when registering functions. > > I know that some may say, "just use lowercase class name for the > constructor in the function entry list", but that messes with nice > class/function names again. > > -Andrei http://www.gravitonic.com/ > * It said 'Winmodem' on the box, but I still feel like I lost. *
> Index: zend_API.c > =================================================================== > RCS file: /repository/ZendEngine2/zend_API.c,v > retrieving revision 1.158 > diff -u -2 -b -w -B -r1.158 zend_API.c > --- zend_API.c 13 Mar 2003 20:41:58 -0000 1.158 > +++ zend_API.c 18 Mar 2003 21:49:31 -0000 > @@ -1147,4 +1147,6 @@ > int error_type; > zend_function *ctor = NULL, *dtor = NULL, *clone = NULL; > + char *lowercase_name; > + int fname_len; > > if (type==MODULE_PERSISTENT) { > @@ -1171,6 +1174,10 @@ > return FAILURE; > } > - if (zend_hash_add(target_function_table, ptr->fname, strlen(ptr->fname)+1, &function, sizeof(zend_function), (void**)&reg_function) == FAILURE) { > + fname_len = strlen(ptr->fname); > + lowercase_name = zend_strndup(ptr->fname, fname_len); > + zend_str_tolower(lowercase_name, fname_len); > + if (zend_hash_add(target_function_table, lowercase_name, fname_len+1, &function, sizeof(zend_function), (void**)&reg_function) == FAILURE) { > unload=1; > + free(lowercase_name); > break; > } > @@ -1192,4 +1199,5 @@ > ptr++; > count++; > + free(lowercase_name); > } > if (unload) { /* before unloading, display all remaining bad function in the module */ >
> -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php
-Andrei http://www.gravitonic.com/ * "UNIX, isn't that some archaic form of DOS?" - our job applicant *

Sterling Hughes

23 years ago
On Tue, 2003-03-18 at 16:57, Andrei Zmievski wrote:
> Grr. Totally slipped my mind that with this patch, the original > function names will be lost when doing get_class_methods() and similar > stuff. Any other suggestions? >
Don't do that. :) The other problem is (besides what derick said) with Ze2 it is not assured that accesses will be lowercased, that's an optional thing. One can overload classes, and be CS (I do it with ext/mono). My introspection solution would allow you to keep proper names btw. :) -Sterling
> On Tue, 18 Mar 2003, Andrei Zmievski wrote: > > Currently when registering functions, the unmodified function name is > > used as the key for the function table. This leads to problems when an > > extension class has a constructor that is not all lowercased. For > > example: > > > > function_entry php_gtk_button_functions[] = { > > PHP_FE("GtkButton", NULL), > > ... > > }; > > > > Then when using something like: > > > > ... > > parent::GtkButton(); > > ... > > > > It fails because it uses 'gtkbutton' as the key to search the function > > table. The attached patch fixes that by lowercasing the key (function > > name) when registering functions. > > > > I know that some may say, "just use lowercase class name for the > > constructor in the function entry list", but that messes with nice > > class/function names again. > > > > -Andrei http://www.gravitonic.com/ > > * It said 'Winmodem' on the box, but I still feel like I lost. * > > > Index: zend_API.c > > =================================================================== > > RCS file: /repository/ZendEngine2/zend_API.c,v > > retrieving revision 1.158 > > diff -u -2 -b -w -B -r1.158 zend_API.c > > --- zend_API.c 13 Mar 2003 20:41:58 -0000 1.158 > > +++ zend_API.c 18 Mar 2003 21:49:31 -0000 > > @@ -1147,4 +1147,6 @@ > > int error_type; > > zend_function *ctor = NULL, *dtor = NULL, *clone = NULL; > > + char *lowercase_name; > > + int fname_len; > > > > if (type==MODULE_PERSISTENT) { > > @@ -1171,6 +1174,10 @@ > > return FAILURE; > > } > > - if (zend_hash_add(target_function_table, ptr->fname, strlen(ptr->fname)+1, &function, sizeof(zend_function), (void**)&reg_function) == FAILURE) { > > + fname_len = strlen(ptr->fname); > > + lowercase_name = zend_strndup(ptr->fname, fname_len); > > + zend_str_tolower(lowercase_name, fname_len); > > + if (zend_hash_add(target_function_table, lowercase_name, fname_len+1, &function, sizeof(zend_function), (void**)&reg_function) == FAILURE) { > > unload=1; > > + free(lowercase_name); > > break; > > } > > @@ -1192,4 +1199,5 @@ > > ptr++; > > count++; > > + free(lowercase_name); > > } > > if (unload) { /* before unloading, display all remaining bad function in the module */ > > > > > -- > > PHP Internals - PHP Runtime Development Mailing List > > To unsubscribe, visit: http://www.php.net/unsub.php > > > > -Andrei http://www.gravitonic.com/ > * "UNIX, isn't that some archaic form of DOS?" - our job applicant *
-- Good judgement comes from experience, and experience comes from bad judgement. - Fred Brooks

Andrei Zmievski

23 years ago
On Tue, 18 Mar 2003, Sterling Hughes wrote:
> Don't do that. :) > > The other problem is (besides what derick said) with Ze2 it is not > assured that accesses will be lowercased, that's an optional thing. One > can overload classes, and be CS (I do it with ext/mono).
I realize that I can overload get_constructor and get_method handlers and do all that stuff. But, seriously, this is screwed up. Given the situation where class is "GtkButton", if I specify constructor all lowercased, it won't be called at all! If I make it the same as the class name, then "parent::GtkButton" stuff stops working. Something must be done. -Andrei http://www.gravitonic.com/ "Everything is a matter of a little programming" -- Rasmus Lerdorf

Sterling Hughes

23 years ago
On Tue, 2003-03-18 at 17:10, Andrei Zmievski wrote:
> On Tue, 18 Mar 2003, Sterling Hughes wrote: > > Don't do that. :) > > > > The other problem is (besides what derick said) with Ze2 it is not > > assured that accesses will be lowercased, that's an optional thing. One > > can overload classes, and be CS (I do it with ext/mono). > > I realize that I can overload get_constructor and get_method handlers > and do all that stuff. But, seriously, this is screwed up. Given the > situation where class is "GtkButton", if I specify constructor all > lowercased, it won't be called at all! If I make it the same as the > class name, then "parent::GtkButton" stuff stops working. Something must > be done.
You're missing my point. Your patch doesn't allow for one to use get_constructor and get_method handlers to implement case-sensitive handlers, as you are lowercasing the stuff on insert, the patch is limiting. I think the best solution is just to play nice with the engine, register it as "gtkbutton" or "__constructor" in the code itself. Then, add an introspection callback to the engine, which can handle all the accesses to function tables, etc. -Sterling
> -Andrei http://www.gravitonic.com/ > > "Everything is a matter of a little programming" -- Rasmus Lerdorf
-- -- "That stuff's easy compared to installing horde :)" - Alan Knowles, In response to my applause for creating a LALR parser for PHP.

Derick Rethans

23 years ago
On Tue, 18 Mar 2003, Sterling Hughes wrote:
> On Tue, 2003-03-18 at 17:10, Andrei Zmievski wrote: > > > > I realize that I can overload get_constructor and get_method handlers > > and do all that stuff. But, seriously, this is screwed up. Given the > > situation where class is "GtkButton", if I specify constructor all > > lowercased, it won't be called at all! If I make it the same as the > > class name, then "parent::GtkButton" stuff stops working. Something must > > be done. > > You're missing my point. Your patch doesn't allow for one to use > get_constructor and get_method handlers to implement case-sensitive > handlers, as you are lowercasing the stuff on insert, the patch is > limiting. > > I think the best solution is just to play nice with the engine, register > it as "gtkbutton" or "__constructor" in the code itself. Then, add an > introspection callback to the engine, which can handle all the accesses > to function tables, etc.
But that doesn't fix the problem, only works around it... Derick
-- "my other box is your windows PC" ------------------------------------------------------------------------- Derick Rethans http://derickrethans.nl/ PHP Magazine - PHP Magazine for Professionals http://php-mag.net/ -------------------------------------------------------------------------

Andrei Zmievski

23 years ago
On Tue, 18 Mar 2003, Derick Rethans wrote:
> > You're missing my point. Your patch doesn't allow for one to use > > get_constructor and get_method handlers to implement case-sensitive > > handlers, as you are lowercasing the stuff on insert, the patch is > > limiting. > > > > I think the best solution is just to play nice with the engine, register > > it as "gtkbutton" or "__constructor" in the code itself. Then, add an > > introspection callback to the engine, which can handle all the accesses > > to function tables, etc. > > But that doesn't fix the problem, only works around it...
We really need to discuss this issue. Maybe at the Montreal conference, in person. Because this case-sensitivity problem is going to poke its ugly head up again and again. -Andrei http://www.gravitonic.com/ * Unix is user friendly, it is just chooses its users selectively. *

Andrei Zmievski

23 years ago
On Tue, 18 Mar 2003, Sterling Hughes wrote:
> You're missing my point. Your patch doesn't allow for one to use > get_constructor and get_method handlers to implement case-sensitive > handlers, as you are lowercasing the stuff on insert, the patch is > limiting.
I already said that my patch sucks, did you miss that?
> I think the best solution is just to play nice with the engine, register > it as "gtkbutton" or "__constructor" in the code itself. Then, add an > introspection callback to the engine, which can handle all the accesses > to function tables, etc.
That approach has drawbacks. Using "__construct" means breaking BC for people using my extension, and registering constructor for "GtkButton" class as "gtkbutton" doesn't work - the constructor is not called. -Andrei http://www.gravitonic.com/ "Perl - the only language that looks the same before and after RSA encryption." -Keith Bostic

Sterling Hughes

23 years ago
On Tue, 2003-03-18 at 17:21, Andrei Zmievski wrote:
> On Tue, 18 Mar 2003, Sterling Hughes wrote: > > You're missing my point. Your patch doesn't allow for one to use > > get_constructor and get_method handlers to implement case-sensitive > > handlers, as you are lowercasing the stuff on insert, the patch is > > limiting. > > I already said that my patch sucks, did you miss that? >
yep :)
> > I think the best solution is just to play nice with the engine, register > > it as "gtkbutton" or "__constructor" in the code itself. Then, add an > > introspection callback to the engine, which can handle all the accesses > > to function tables, etc. > > That approach has drawbacks. Using "__construct" means breaking BC for > people using my extension, and registering constructor for "GtkButton" > class as "gtkbutton" doesn't work - the constructor is not called. >
ahh, that does indeed suck, perhaps there should be a special case in the case sensitivity rules regarding constructors? -Sterling PS: or, *gasp* drop case-sensitivity. For which I'm an enormous +1. :)
-- "Programming today is a race between software engineers stirring to build bigger and better idiot-proof programs, and the universe trying to produce bigger and better idiots. So far, the universe is winning." - Unknown

Ronald Chmara

23 years ago
On Tuesday, March 18, 2003, at 03:07 PM, Sterling Hughes wrote:
> PS: or, *gasp* drop case-sensitivity. For which I'm an enormous +1. :)
Aw, and steal enormous hours from consultants like me who make mad dollars from running grep -i <misbehaving function>? How cruel. :-) -Bop

(Marcus Börger)

23 years ago
At 22:57 18.03.2003, Derick Rethans wrote:
>On Tue, 18 Mar 2003, Andrei Zmievski wrote: > > > Currently when registering functions, the unmodified function name is > > used as the key for the function table. This leads to problems when an > > extension class has a constructor that is not all lowercased. For > > example: > > > > [...] > > > > I know that some may say, "just use lowercase class name for the > > constructor in the function entry list", but that messes with nice > > class/function names again. > >Maybe we can finally fix the OO model and just make them case-sensitive? :) >If that's not an option, I think the patch is OK.
Ok before making PHP case-sensitiv i also agree to your patch even though adding any strlowering at c level slows down the engine... marcus

Andi Gutmans

23 years ago
If the memory manager is usable at this point (I think it is) then you should use do_alloca() and free_alloca() instead of malloc()/free(). Andi At 04:54 PM 3/18/2003 -0500, Andrei Zmievski wrote:

Andrei Zmievski

23 years ago
On Wed, 19 Mar 2003, Andi Gutmans wrote:
> If the memory manager is usable at this point (I think it is) then you > should use do_alloca() and free_alloca() instead of malloc()/free().
That's a good tip. But what do you think about the issue in general? On one hand, lowercasing function names/keys will destroy the original case, which the author may want to preserve. Without it though, there are issues with constructors and such.. -Andrei http://www.gravitonic.com/ 'Any given program, when running correctly, is obsolete.' - First Law of Computer Programming