Re: Zend Constants PATCH

Hey, Please test the attached patch and let me know if it works for you. I haven't really had the chance to test it too much. Andi At 06:21 PM 6/15/2002 -0400, Ilia A. wrote: >Andi, > >I wrote another patch, this time a 'proper' way, which means the old >functionality of case insensetivity is supported. Please look it over, >hopefuly this is good enough to commit. >I've also attached a small php test script you can use to see the problem in >non patched PHPs. > >Ilia > >On June 15, 2002 03:25 pm, Andi Gutmans wrote: > > Ilia, > > > > I remember now the problem you're talking about. It has been discussed here > > in the past and I don't recall us having found a good solution. Basically > > we need a solution which is backwards compatible but will allow "TEST" and > > "test" to co-exist if case sensitivity was chosen for them. > > It's something to think about and not create a quick 2 line patch for the > > problem. I think one of the suggestions was using two hash tables. First > > doing a case-sensitive lookup and only if the constant isn't found doing a > > case-insensitive lookup. > > > > Andi > > > > At 03:40 PM 6/15/2002 -0400, Ilia A. wrote: > > >Andi, > > > > > >Yes, you are correct in that respect, my patch would accomplish just that. > > >No where in PHP documentation does it say that you cannot have TEST and > > > test defines in the same script. Unless you specifically tell the > > > define() function to treat the define as case insensitive. > > >Because the defines are always lowercased unless the defines for i18n > > > systems are always declared in lower case any define with a letter 'I' > > > for example would break on a system using most non English locales. This > > > is a VERY serious problems, for example consider the reversal of the > > > htmlenteties() function. The following code: > > >get_html_translation_table (HTML_ENTITIES); > > >will break if a ru_UI or tr_TR or any other number of non-English locales > > > are exported. > > > > > >In addition because all locales are lower cased defines suffer large > > >performance degradation when compared to other variables because another > > > copy of the define name needs to be allocated and then lower cased every > > > single time a define is declared or retrieved. > > > > > >As far as I know, php variables are always case sensitive and there is now > > >way > > >to make them not, why an exception was made for defines I do not know, > > >especially when you consider that in C and C++ defines are ALWAYS case > > >sensitive. IMHO this is a very bad feature, that not only implements > > > useless functionality but actually causes PHP code to break. > > >Therefor, I humbly ask that you reconsider your position on this issue. > > > > > > > > >Ilia > > > > > >On June 15, 2002 03:03 pm, you wrote: > > > > Ilia, > > > > > > > > Your patch basically makes PHP constants case sensitive. > > > > Changing this is a very big backwards compatibility problem. > > > > You're not supposed to register two define's with the same letters but > > > > different case. > > > > > > > > Andi > > > > > > > > At 01:21 PM 6/15/2002 -0400, Ilia A. wrote: > > > > >Hello, > > > > > > > > > >While developing software in PHP that supports i18n I've come across > > > > > several problems that affect defines made in PHP. > > > > >The first problem is that when a define is declared and its name > > > > > contains upper case characters such as I, the define becomes unusable > > > > > if a locale, which does not support those chracters is exported, such > > > > > as tr_TR or ru_IU. Bug Report at: > > > > > http://bugs.php.net/bug.php?id=16865 > > > > > > > > > >There is a problem with case sensetivity of defines, for example, if > > > > > you create a case sensetive define 'TEST' and then a case > > > > > sensetive > > > > > define 'test', the latter define's value will be lost. > > > > >Bug Report at: http://bugs.php.net/?id=17658 > > > > > > > > > >The problem occurs because zend internally (zend_constants.c) seems to > > > > > always lowecase the define before it is fetched/added to the hash > > > > > table of defines. This causes problem for i18n because the define is > > > > > lowercased using c's tolower function, which is affected by locale > > > > > settings. Because it is stored as lower case, having 2 defines with > > > > > the same name but in different case also becomes impossible to do. > > > > > > > > > >Attached is a patch against zend_constants.c CVS revision 1.38 that > > > > > fixes both > > > > >of these bugs, I hope the developers would consider adding this patch > > > > > to the CVS. > > > > > > > > > >Ilia > > > > >-- > > > > >PHP Development Mailing List > > > > ><http://www.php.net/> > > > > >To unsubscribe, visit: > > > > >http://www.php.net/unsub.php >

Index: zend_constants.c =================================================================== RCS file: /repository/ZendEngine2/zend_constants.c,v retrieving revision 1.39 diff -u -r1.39 zend_constants.c --- zend_constants.c 6 Jan 2002 15:21:35 -0000 1.39 +++ zend_constants.c 18 Jun 2002 19:10:38 -0000 @@ -220,26 +220,27 @@ { zend_constant *c; char *lookup_name; - int retval; + int retval = 1; - lookup_name = do_alloca(name_len+1); - memcpy(lookup_name, name, name_len+1); - - zend_str_tolower(lookup_name, name_len); - - if (zend_hash_find(EG(zend_constants), lookup_name, name_len+1, (void **) &c)==SUCCESS) { - if ((c->flags & CONST_CS) && memcmp(c->name, name, name_len)!=0) { - retval=0; + if (zend_hash_find(EG(zend_constants), name, name_len+1, (void **) &c) == FAILURE) { + lookup_name = do_alloca(name_len+1); + memcpy(lookup_name, name, name_len+1); + zend_str_tolower(lookup_name, name_len); + + if (zend_hash_find(EG(zend_constants), lookup_name, name_len+1, (void **) &c)==SUCCESS) { + if ((c->flags & CONST_CS) && memcmp(c->name, name, name_len)!=0) { + retval=0; + } } else { - retval=1; - *result = c->value; - zval_copy_ctor(result); + retval=0; } - } else { - retval=0; + free_alloca(lookup_name); } - free_alloca(lookup_name); + if (retval) { + *result = c->value; + zval_copy_ctor(result); + } return retval; } @@ -247,14 +248,21 @@ ZEND_API int zend_register_constant(zend_constant *c TSRMLS_DC) { - char *lowercase_name = zend_strndup(c->name, c->name_len); + char *lowercase_name; int ret = SUCCESS; #if 0 printf("Registering constant for module %d\n", c->module_number); #endif - zend_str_tolower(lowercase_name, c->name_len); + lowercase_name = do_alloca(c->name_len); + + memcpy(lowercase_name, c->name, c->name_len); + + if (!(c->flags & CONST_CS)) { + zend_str_tolower(lowercase_name, c->name_len); + } + if (zend_hash_add(EG(zend_constants), lowercase_name, c->name_len, (void *) c, sizeof(zend_constant), NULL)==FAILURE) { free(c->name); if (!(c->flags & CONST_PERSISTENT) @@ -264,7 +272,7 @@ zend_error(E_NOTICE,"Constant %s already defined", lowercase_name); ret = FAILURE; } - free(lowercase_name); + free_alloca(lowercase_name); return ret; }