# Translating optional zval ** with zend_parse_parameters() to PHP 7 - Thread: https://externals.io/message/97167 - Original archive: https://news-web.php.net/php.internals/97167 - Started by Thomas Hruska on 2016-11-25T03:13:21+00:00 - Messages: 10 - 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 --- ## #97167 — Thomas Hruska — 2016-11-25T03:13:21+00:00 URL: https://externals.io/message/97167#97167 I'm working on updating an extension for PHP 7 compatibility. I have one function that uses an optional zval ** with zend_parse_parameters(). zval **zprevcount = NULL; int count; int argc = ZEND_NUM_ARGS(); if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "|Z", &zprevcount) == FAILURE) return; ... if (argc > 0) { count = (int)PrevCount; zval_dtor(*zprevcount); ZVAL_LONG(*zprevcount, count); } What's the correct way to translate that into PHP 7? -- Thomas Hruska CubicleSoft President I've got great, time saving software that you will find useful. http://cubiclesoft.com/ --- ## #97170 — Kalle Sommer Nielsen — 2016-11-25T07:27:24+00:00 URL: https://externals.io/message/97167#97170 · In reply to: #97167 Hi Thomas 2016-11-25 4:13 GMT+01:00 Thomas Hruska : > I'm working on updating an extension for PHP 7 compatibility. I have one > function that uses an optional zval ** with zend_parse_parameters(). > > zval **zprevcount = NULL; > int count; > int argc = ZEND_NUM_ARGS(); > > if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "|Z", > &zprevcount) == FAILURE) return; > > ... > > if (argc > 0) > { > count = (int)PrevCount; > > zval_dtor(*zprevcount); > ZVAL_LONG(*zprevcount, count); > } > > What's the correct way to translate that into PHP 7? Since you are working with integers, you can do: zend_long prevcount = 0; if(zend_parse_parameters(ZEND_NUM_ARGS(), "|l", &prevcount) == FAILURE) { return; } if(prevcount) { /* ... use prevcount ... */ } Notice that all integers returned from the ZPP is a zend_long, you can see more information about the ZPP in php-src/README.PARAMETER_PARSING_API -- regards, Kalle Sommer Nielsen kalle@php.net --- ## #97180 — Thomas Hruska — 2016-11-25T12:30:33+00:00 URL: https://externals.io/message/97167#97180 · In reply to: #97170 > Hi Thomas > > 2016-11-25 4:13 GMT+01:00 Thomas Hruska : > > […] > > Since you are working with integers, you can do: > > zend_long prevcount = 0; > > if(zend_parse_parameters(ZEND_NUM_ARGS(), "|l", &prevcount) == FAILURE) > { > return; > } > > if(prevcount) > { > /* ... use prevcount ... */ > } > > Notice that all integers returned from the ZPP is a zend_long, you can > see more information about the ZPP in > php-src/README.PARAMETER_PARSING_API Unless I missed something, zend_long is generally defined as a standard int64_t or int32_t type (Zend/zend_long.h). I need to be able to *modify* the original variable that was passed in. I already use "|l" and zend_long elsewhere in the extension. I'm thinking the correct ZPP string is "|z/" but the usage examples in core that I've looked at so far are inconsistent, hence the original question of what's the correct approach? -- Thomas Hruska CubicleSoft President I've got great, time saving software that you will find useful. http://cubiclesoft.com/ --- ## #97192 — Kalle Sommer Nielsen — 2016-11-25T20:43:02+00:00 URL: https://externals.io/message/97167#97192 · In reply to: #97180 2016-11-25 13:30 GMT+01:00 Thomas Hruska : > I need to be able to *modify* the original variable that was passed in. I already use "|l" and zend_long > elsewhere in the extension Ah my bad I misread it! In that case take a look at where such is implemented in php-src, `dns_get_record()` is an example despite the slightly clouded code: http://git.php.net/?p=php-src.git;a=blob;f=ext/standard/dns.c;h=f92015eee90d3e93a801e93d6381d89923825166;hb=refs/heads/master#l1011 See $weight_list: - Check if the argument is passed - zval_dtor() - Populate it (ZVAL_LONG is the same) The main difference from your code is to use 'z' (lower case) in ZPP, and then use zval* (which is what 'z' returns). I'm not at my dev machine, so I cannot test it but that should be how it goes -- regards, Kalle Sommer Nielsen kalle@php.net --- ## #97194 — Jakub Zelenka — 2016-11-25T23:32:49+00:00 URL: https://externals.io/message/97167#97194 · In reply to: #97192 2016-11-25 13:30 GMT+01:00 Thomas Hruska : > I need to be able to *modify* the original variable that was passed in. I already use "|l" and zend_long > elsewhere in the extension Ah my bad I misread it! In that case take a look at where such is implemented in php-src, `dns_get_record()` is an example despite the slightly clouded code: http://git.php.net/?p=php-src.git;a=blob;f=ext/standard/dns.c;h= f92015eee90d3e93a801e93d6381d89923825166;hb=refs/heads/master#l1011 See $weight_list: - Check if the argument is passed - zval_dtor() - Populate it (ZVAL_LONG is the same) The main difference from your code is to use 'z' (lower case) in ZPP, and then use zval* (which is what 'z' returns). You also need set 1 for the first param of ZEND_ARG_INFO (specify that it is a ref param) otherwise it won't use the changed val... Jakub --- ## #97195 — Thomas Hruska — 2016-11-26T00:47:14+00:00 URL: https://externals.io/message/97167#97195 · In reply to: #97194 > > > 2016-11-25 13:30 GMT+01:00 Thomas Hruska : > > […] > I already use "|l" and zend_long > > […] > > Ah my bad I misread it! > > In that case take a look at where such is implemented in php-src, > `dns_get_record()` is an example despite the slightly clouded code: > http://git.php.net/?p=php-src.git;a=blob;f=ext/standard/dns.c;h= > f92015eee90d3e93a801e93d6381d89923825166;hb=refs/heads/master#l1011 > > See $weight_list: > > - Check if the argument is passed > - zval_dtor() > - Populate it (ZVAL_LONG is the same) > > The main difference from your code is to use 'z' (lower case) in ZPP, > and then use zval* (which is what 'z' returns). > > > > You also need set 1 for the first param of ZEND_ARG_INFO (specify that it > is a ref param) otherwise it won't use the changed val... > > Jakub Okay, everyone has been helpful. Thanks. I'll go with: zval *zprevcount = NULL; zend_long count; if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "|z/", &zprevcount) == FAILURE) return; ... if (zprevcount != NULL) { count = (zend_long)PrevCount; zval_dtor(zprevcount); ZVAL_LONG(zprevcount, count); } Just one little thing I found in php_str_replace_common() in ext/standard/string.c. That particular function calls zval_ptr_dtor() instead of zval_dtor() on zcount. Just wondering why it does that - I'm thinking it has something to do with removing the ZPP call for PHP 7 and using the (mostly undocumented?) Z_PARAM_ZVAL_EX() macro. However, php_do_pcre_match() over in ext/pcre/php_pcre.c calls zval_dtor() on subpats but also uses the Z_PARAM_ZVAL_EX() macro in a similar fashion. Looking back at PHP 5.6's php_str_replace_common() shows that it previously called zval_dtor(). So it might also be a bug of some sort. -- Thomas Hruska CubicleSoft President I've got great, time saving software that you will find useful. http://cubiclesoft.com/ --- ## #97196 — Christoph M. Becker — 2016-11-26T11:29:47+00:00 URL: https://externals.io/message/97167#97196 · In reply to: #97195 > Okay, everyone has been helpful. Thanks. I'll go with: > > > zval *zprevcount = NULL; > zend_long count; > > if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "|z/", > &zprevcount) == FAILURE) return; > > ... > > if (zprevcount != NULL) > { > count = (zend_long)PrevCount; > > zval_dtor(zprevcount); > ZVAL_LONG(zprevcount, count); > } > > > Just one little thing I found in php_str_replace_common() in > ext/standard/string.c. That particular function calls zval_ptr_dtor() > instead of zval_dtor() on zcount. Just wondering why it does that - I'm > thinking it has something to do with removing the ZPP call for PHP 7 and > using the (mostly undocumented?) Z_PARAM_ZVAL_EX() macro. However, > php_do_pcre_match() over in ext/pcre/php_pcre.c calls zval_dtor() on > subpats but also uses the Z_PARAM_ZVAL_EX() macro in a similar fashion. > Looking back at PHP 5.6's php_str_replace_common() shows that it > previously called zval_dtor(). So it might also be a bug of some sort. zval_dtor() destroys the value; zval_ptr_dtor() decrements the refcount, and destroys the value only if the refcount has dropped to 0. See also (which is written for PHP 5, but is still useful). In your case, zval_dtor() is appropriate, as the zval has already been separated (ZPP's /), so there can't be other references. -- Christoph M. Becker --- ## #97197 — Nikita Popov — 2016-11-26T11:45:45+00:00 URL: https://externals.io/message/97167#97197 · In reply to: #97196 On Sat, Nov 26, 2016 at 12:29 PM, Christoph M. Becker wrote: > > > > […] > > zval_dtor() destroys the value; zval_ptr_dtor() decrements the refcount, > and destroys the value only if the refcount has dropped to 0. See also > (which is > written for PHP 5, but is still useful). > > In your case, zval_dtor() is appropriate, as the zval has already been > separated (ZPP's /), so there can't be other references. The situation here has become a bit confusing in PHP 7. zval_dtor() is now actually the same as zval_ptr_dtor_nogc(). As such, you can use zval_dtor() on zvals with rc>1, but collectible zvals will not be added to the GC root buffer. In the vast majority of cases, you will want to use zval_ptr_dtor(). zval_dtor() / zval_ptr_dtor_nogc() are useful either as an optimization in cases where you know for certain that a value is a root, or to prevent values from being placed in the GC root buffer, in cases where this violates the memory model (e.g. this is relevant for opcache'd literals). The fact that php_pcre.c uses zval_dtor() is simply a bug, because code like $obj = new stdClass; $obj->obj = $obj; preg_match('/./', 'x', $obj); leaks. Nikita --- ## #97198 — Christoph M. Becker — 2016-11-26T12:55:34+00:00 URL: https://externals.io/message/97167#97198 · In reply to: #97197 > On Sat, Nov 26, 2016 at 12:29 PM, Christoph M. Becker > wrote: > > > […] > > The situation here has become a bit confusing in PHP 7. zval_dtor() is now > actually the same as zval_ptr_dtor_nogc(). As such, you can use zval_dtor() > on zvals with rc>1, but collectible zvals will not be added to the GC root > buffer. > > In the vast majority of cases, you will want to use zval_ptr_dtor(). > zval_dtor() / zval_ptr_dtor_nogc() are useful either as an optimization in > cases where you know for certain that a value is a root, or to prevent > values from being placed in the GC root buffer, in cases where this > violates the memory model (e.g. this is relevant for opcache'd literals). Thanks for the explanation, Nikita. > The fact that php_pcre.c uses zval_dtor() is simply a bug, because code like > > $obj = new stdClass; > $obj->obj = $obj; > preg_match('/./', 'x', $obj); > > leaks. Indeed. At least `preg_replace()`'s $count parameter has the same issue. Is there already a bug report about this? -- Christoph M. Becker --- ## #97199 — Christoph M. Becker — 2016-11-26T14:14:26+00:00 URL: https://externals.io/message/97167#97199 · In reply to: #97198 > > > > […] > > Indeed. At least `preg_replace()`'s $count parameter has the same issue. > Is there already a bug report about this? FTR: reported as . -- Christoph M. Becker