WIKI: phpng-upgrading

php.internals

Yasuo Ohgaki

10 years ago
Hi ZendEngine developers, I'm not sure if the wiki page is maintained, but I noticed few errors. https://wiki.php.net/phpng-upgrading#strings has following example. - ZVAL_STRING(zv, str, 1); + ZVAL_STRING(zv, str); - ZVAL_STRINGL(zv, str, len, 1); + ZVAL_STRINGL(zv, str, len); - ZVAL_STRING(zv, str, 0); + ZVAL_STRING(zv, str); + efree(str); - ZVAL_STRINGL(zv, str, len, 0); + ZVAL_STRINGL(zv, str, len); + efree(str); Since PHP5 has following definition for ZVAL_STRING*() #define ZVAL_STRING(z, s, duplicate) do { \ const char *__s=(s); \ zval *__z = (z); \ Z_STRLEN_P(__z) = strlen(__s); \ Z_STRVAL_P(__z) = (duplicate?estrndup(__s, Z_STRLEN_P(__z)):(char*)__s);\ Z_TYPE_P(__z) = IS_STRING; \ } while (0) the example's 0 and 1 are flipped, efree() locations are wrong. In https://wiki.php.net/phpng-upgrading#zend_string_api , it says zend_string_init(char *val, int len, int persistent) Current definition is zend_string_init(char *val, size_t len, int persistent) Should I update the doc or is the doc is maintained elsewhere? Regards
-- Yasuo Ohgaki yohgaki@ohgaki.net

Sean DuBois

10 years ago
On Thu, Jan 21, 2016 at 06:55:41AM +0900, Yasuo Ohgaki wrote:
> Hi ZendEngine developers, > > I'm not sure if the wiki page is maintained, but I noticed few errors. > > https://wiki.php.net/phpng-upgrading#strings > has following example. > > - ZVAL_STRING(zv, str, 1); > + ZVAL_STRING(zv, str); > > - ZVAL_STRINGL(zv, str, len, 1); > + ZVAL_STRINGL(zv, str, len); > > - ZVAL_STRING(zv, str, 0); > + ZVAL_STRING(zv, str); > + efree(str); > > - ZVAL_STRINGL(zv, str, len, 0); > + ZVAL_STRINGL(zv, str, len); > + efree(str); > > Since PHP5 has following definition for ZVAL_STRING*() > > #define ZVAL_STRING(z, s, duplicate) do { \ > const char *__s=(s); \ > zval *__z = (z); \ > Z_STRLEN_P(__z) = strlen(__s); \ > Z_STRVAL_P(__z) = (duplicate?estrndup(__s, > Z_STRLEN_P(__z)):(char*)__s);\ > Z_TYPE_P(__z) = IS_STRING; \ > } while (0) > > the example's 0 and 1 are flipped, efree() locations are wrong. > > In https://wiki.php.net/phpng-upgrading#zend_string_api , > it says > > zend_string_init(char *val, int len, int persistent) > > Current definition is > > zend_string_init(char *val, size_t len, int persistent) > > Should I update the doc or is the doc is maintained > elsewhere? > > Regards > > -- > Yasuo Ohgaki > yohgaki@ohgaki.net > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php >
Hi! I tried to get access to that page as well, but didn't have any luck would you mind adding the zend_parse_paramaters changes? 'l' went from 'long' -> 'zend_long' and 's' went from 'int' -> 'size_t'. So many extensions have been ported without this in mind, and it bites in really nasty hard to reproduce runtime ways. thanks!

Yasuo Ohgaki

10 years ago
Hi Sean, On Thu, Jan 21, 2016 at 7:07 AM, Sean DuBois <sean@siobud.com> wrote:
> > I tried to get access to that page as well, but didn't have any luck > would you mind adding the zend_parse_paramaters changes? > > 'l' went from 'long' -> 'zend_long' and 's' went from 'int' -> 'size_t'. > > So many extensions have been ported without this in mind, and it bites > in really nasty hard to reproduce runtime ways.
Good point. zend_parse_parameters() is using "size_t" for string length, since zend_string uses size_t for it. struct _zend_string { zend_refcounted_h gc; zend_ulong h; /* hash value */ size_t len; char val[1]; }; AFAIK, PHP7 repo's function parameters are changed to size_t. This should be described in upgrading doc for 3rd party module developers. Regards,
-- Yasuo Ohgaki yohgaki@ohgaki.net

François Laupretre

10 years ago
Le 20/01/2016 23:07, Sean DuBois a écrit :
> On Thu, Jan 21, 2016 at 06:55:41AM +0900, Yasuo Ohgaki wrote: >> Hi ZendEngine developers, >> >> I'm not sure if the wiki page is maintained, but I noticed few errors. >> >> https://wiki.php.net/phpng-upgrading#strings >> has following example. >> >> - ZVAL_STRING(zv, str, 1); >> + ZVAL_STRING(zv, str); >> >> - ZVAL_STRINGL(zv, str, len, 1); >> + ZVAL_STRINGL(zv, str, len); >> >> - ZVAL_STRING(zv, str, 0); >> + ZVAL_STRING(zv, str); >> + efree(str); >> >> - ZVAL_STRINGL(zv, str, len, 0); >> + ZVAL_STRINGL(zv, str, len); >> + efree(str); >> >> Since PHP5 has following definition for ZVAL_STRING*() >> >> #define ZVAL_STRING(z, s, duplicate) do { \ >> const char *__s=(s); \ >> zval *__z = (z); \ >> Z_STRLEN_P(__z) = strlen(__s); \ >> Z_STRVAL_P(__z) = (duplicate?estrndup(__s, >> Z_STRLEN_P(__z)):(char*)__s);\ >> Z_TYPE_P(__z) = IS_STRING; \ >> } while (0) >> >> the example's 0 and 1 are flipped, efree() locations are wrong. >> >> In https://wiki.php.net/phpng-upgrading#zend_string_api , >> it says >> >> zend_string_init(char *val, int len, int persistent) >> >> Current definition is >> >> zend_string_init(char *val, size_t len, int persistent) >> >> Should I update the doc or is the doc is maintained >> elsewhere? >> >> Regards >> >> -- >> Yasuo Ohgaki >> yohgaki@ohgaki.net >> >> -- >> PHP Internals - PHP Runtime Development Mailing List >> To unsubscribe, visit: http://www.php.net/unsub.php >> > > Hi! > > I tried to get access to that page as well, but didn't have any luck > would you mind adding the zend_parse_paramaters changes? > > 'l' went from 'long' -> 'zend_long' and 's' went from 'int' -> 'size_t'. > > So many extensions have been ported without this in mind, and it bites > in really nasty hard to reproduce runtime ways. > > thanks! >
May I suggest we also include in this document a pointer to the PECL compatibility library (https://github.com/flaupretre/pecl-compat), a compatibility layer for PHP 5 & 7. Of course, this is not solving every issues but it can help a lot, especially on string/array-related issues. Among others, it includes a solution to the zend_parse_parameters() issue (using COMPAT_ARG_SIZE_T/COMPAT_ARG_LONG_T as variable types). Regards François

Nikita Popov

10 years ago
On Wed, Jan 20, 2016 at 11:07 PM, Sean DuBois <sean@siobud.com> wrote:
> Hi! > > I tried to get access to that page as well, but didn't have any luck > would you mind adding the zend_parse_paramaters changes? > > 'l' went from 'long' -> 'zend_long' and 's' went from 'int' -> 'size_t'. > > So many extensions have been ported without this in mind, and it bites > in really nasty hard to reproduce runtime ways. >
I've added these two to the zpp section, thanks! Nikita

Nikita Popov

10 years ago
On Wed, Jan 20, 2016 at 10:55 PM, Yasuo Ohgaki <yohgaki@ohgaki.net> wrote:
> Hi ZendEngine developers, > > I'm not sure if the wiki page is maintained, but I noticed few errors. > > https://wiki.php.net/phpng-upgrading#strings > has following example. > > - ZVAL_STRING(zv, str, 1); > + ZVAL_STRING(zv, str); > > - ZVAL_STRINGL(zv, str, len, 1); > + ZVAL_STRINGL(zv, str, len); > > - ZVAL_STRING(zv, str, 0); > + ZVAL_STRING(zv, str); > + efree(str); > > - ZVAL_STRINGL(zv, str, len, 0); > + ZVAL_STRINGL(zv, str, len); > + efree(str); > > Since PHP5 has following definition for ZVAL_STRING*() > > #define ZVAL_STRING(z, s, duplicate) do { \ > const char *__s=(s); \ > zval *__z = (z); \ > Z_STRLEN_P(__z) = strlen(__s); \ > Z_STRVAL_P(__z) = (duplicate?estrndup(__s, > Z_STRLEN_P(__z)):(char*)__s);\ > Z_TYPE_P(__z) = IS_STRING; \ > } while (0) > > the example's 0 and 1 are flipped, efree() locations are wrong. >
These examples are correct. In PHP 7 ZVAL_STRING always duplicates, which is what the 1 parameter used to signify.
> In https://wiki.php.net/phpng-upgrading#zend_string_api , > it says > > zend_string_init(char *val, int len, int persistent) > > Current definition is > > zend_string_init(char *val, size_t len, int persistent) > > Should I update the doc or is the doc is maintained > elsewhere? >
I've fixed this. But yes, you should update the doc, it's not maintained elsewhere. Thanks, Nikita

Yasuo Ohgaki

10 years ago
Hi Nikita, On Thu, Jan 21, 2016 at 7:28 AM, Nikita Popov <nikita.ppv@gmail.com> wrote:
> These examples are correct. In PHP 7 ZVAL_STRING always duplicates, which is > what the 1 parameter used to signify.
Oh, now I see the example intention. Thank you.
-- Yasuo Ohgaki yohgaki@ohgaki.net

Andrew Faulds

10 years ago
Hi Yasuo, Yasuo Ohgaki wrote:
> Hi ZendEngine developers, > > I'm not sure if the wiki page is maintained, but I noticed few errors. > > https://wiki.php.net/phpng-upgrading#strings
phpng-upgrading was written back when it was just phpng and the 64-bit patch wasn't merged. Heck, I don't think phpng was merged into master at that point either. Given it's now needed for PHP 7 migration, maybe we should move it into UPGRADING.INTERNALS or onto the PHP manual? Thanks.
-- Andrea Faulds https://ajf.me/