Re: [PHP-CVS] svn: /php/php-src/trunk/ext/snmp/ snmp.c
btw, you can use valgrind as well with the tests suite. That should help you to catch other possible leaks or bad memory access. On Sat, Feb 26, 2011 at 8:08 PM, Boris Lytochkin lytbo...@php.net wrote: lytboris Sat, 26 Feb 2011 19:08:55 + Revision: http://svn.php.net/viewvc?view=revisionrevision=308710 Log: remove compiler warnings fix various memory leaks seen with --enable-debug Changed paths: U php/php-src/trunk/ext/snmp/snmp.c -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php -- Pierre @pierrejoye | http://blog.thepimp.net | http://www.libgd.org -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-CVS] svn: /php/php-src/trunk/ext/snmp/ snmp.c
Just to indicate that this is not php function return and some other processing (in caller function) follows. On Sat, Feb 26, 2011 at 11:30 PM, Antony Dovgal t...@daylessday.org wrote: - RETURN_FALSE; + RETVAL_FALSE; + return; What's the point? #define RETURN_FALSE { RETVAL_FALSE; return; } -- Wbr, Antony Dovgal --- http://pinba.org - realtime statistics for PHP -- Boris Lytochkin -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-CVS] svn: /php/php-src/trunk/ext/snmp/ snmp.c
- RETURN_FALSE; + RETVAL_FALSE; + return; What's the point? #define RETURN_FALSE{ RETVAL_FALSE; return; } On 02/26/2011 10:08 PM, Boris Lytochkin wrote: lytboris Sat, 26 Feb 2011 19:08:55 + Revision: http://svn.php.net/viewvc?view=revisionrevision=308710 Log: remove compiler warnings fix various memory leaks seen with --enable-debug Changed paths: U php/php-src/trunk/ext/snmp/snmp.c -- Wbr, Antony Dovgal --- http://pinba.org - realtime statistics for PHP -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-CVS] svn: /php/php-src/trunk/ext/snmp/ snmp.c
can you open a bug at bugs.php.net with the patch please? Thanks! On Sat, Feb 26, 2011 at 9:31 PM, Lytochkin Boris lytbo...@gmail.com wrote: It is a pain to run it on FreeBSD, there is a bug in run-tests.php: === --- run-tests.php (revision 308673) +++ run-tests.php (working copy) @@ -567,7 +567,7 @@ case 'm': $leak_check = true; $valgrind_cmd = valgrind --version; - $valgrind_header = system_with_timeout($valgrind_cmd); + $valgrind_header = system_with_timeout($valgrind_cmd, $_ENV); $replace_count = 0; if (!$valgrind_header) { error(Valgrind returned no version info, cannot proceed.\nPlease check if Valgrind is installed.); === Without $_ENV system_with_timeout() will use bogus(system default) PATH environment variable and will not find valgrind executable. After fixing run-tests.php all 27 tests for ext/snmp passed with valgrind enabled. On Sat, Feb 26, 2011 at 10:19 PM, Pierre Joye pierre@gmail.com wrote: btw, you can use valgrind as well with the tests suite. That should help you to catch other possible leaks or bad memory access. On Sat, Feb 26, 2011 at 8:08 PM, Boris Lytochkin lytbo...@php.net wrote: lytboris Sat, 26 Feb 2011 19:08:55 + Revision: http://svn.php.net/viewvc?view=revisionrevision=308710 Log: remove compiler warnings fix various memory leaks seen with --enable-debug Changed paths: U php/php-src/trunk/ext/snmp/snmp.c -- Pierre @pierrejoye | http://blog.thepimp.net | http://www.libgd.org -- Boris Lytochkin -- Pierre @pierrejoye | http://blog.thepimp.net | http://www.libgd.org -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-CVS] svn: /php/php-src/trunk/ext/snmp/ snmp.c
Sure, #54108 On Sat, Feb 26, 2011 at 11:33 PM, Pierre Joye pierre@gmail.com wrote: can you open a bug at bugs.php.net with the patch please? -- Pierre @pierrejoye | http://blog.thepimp.net | http://www.libgd.org -- Boris Lytochkin -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-CVS] svn: /php/php-src/trunk/ext/snmp/ snmp.c
hi, I wonder why you need that as we don't release snmp outside php. I would suggest to either drop these backward compatible checks (cleaner code) or consider to do release through pecl as well and then these checks make sense. Cheers, On Tue, Feb 1, 2011 at 9:20 AM, Boris Lytochkin lytbo...@php.net wrote: lytboris Tue, 01 Feb 2011 08:20:13 + Revision: http://svn.php.net/viewvc?view=revisionrevision=307898 Log: preprocessed changes made in rev.307894: keeping ext/snmp backwards compatible Changed paths: U php/php-src/trunk/ext/snmp/snmp.c Modified: php/php-src/trunk/ext/snmp/snmp.c === --- php/php-src/trunk/ext/snmp/snmp.c 2011-02-01 07:45:30 UTC (rev 307897) +++ php/php-src/trunk/ext/snmp/snmp.c 2011-02-01 08:20:13 UTC (rev 307898) @@ -506,6 +506,9 @@ static zend_object_value php_snmp_object_new(zend_class_entry *class_type TSRMLS_DC) /* {{{ */ { +#if PHP_VERSION_ID 503099 + zval *tmp; +#endif zend_object_value retval; php_snmp_object *intern; @@ -514,7 +517,11 @@ memset(intern-zo, 0, sizeof(php_snmp_object)); zend_object_std_init(intern-zo, class_type TSRMLS_CC); +#if PHP_VERSION_ID 503099 + zend_hash_copy(intern-zo.properties, class_type-default_properties, (copy_ctor_func_t) zval_add_ref,(void *) tmp, sizeof(zval *)); +#else object_properties_init(intern-zo, class_type); +#endif retval.handle = zend_objects_store_put(intern, (zend_objects_store_dtor_t)zend_objects_destroy_object, (zend_objects_free_object_storage_t) php_snmp_object_free_storage, NULL TSRMLS_CC); retval.handlers = (zend_object_handlers *) php_snmp_object_handlers; @@ -1795,9 +1802,13 @@ } /* }}} */ -/* {{{ php_snmp_read_property(zval *object, zval *member, int type) +/* {{{ php_snmp_read_property(zval *object, zval *member, int type[, const zend_literal *key]) Generic object property reader */ +#if PHP_VERSION_ID 503099 +zval *php_snmp_read_property(zval *object, zval *member, int type TSRMLS_DC) +#else zval *php_snmp_read_property(zval *object, zval *member, int type, const zend_literal *key TSRMLS_DC) +#endif { zval tmp_member; zval *retval; @@ -1827,7 +1838,11 @@ } } else { zend_object_handlers * std_hnd = zend_get_std_object_handlers(); +#if PHP_VERSION_ID 503099 + retval = std_hnd-read_property(object, member, type TSRMLS_CC); +#else retval = std_hnd-read_property(object, member, type, key TSRMLS_CC); +#endif } if (member == tmp_member) { @@ -1837,9 +1852,13 @@ } /* }}} */ -/* {{{ php_snmp_write_property(zval *object, zval *member, zval *value) +/* {{{ php_snmp_write_property(zval *object, zval *member, zval *value[, const zend_literal *key]) Generic object property writer */ +#if PHP_VERSION_ID 503099 +void php_snmp_write_property(zval *object, zval *member, zval *value TSRMLS_DC) +#else void php_snmp_write_property(zval *object, zval *member, zval *value, const zend_literal *key TSRMLS_DC) +#endif { zval tmp_member; php_snmp_object *obj; @@ -1866,7 +1885,11 @@ } } else { zend_object_handlers * std_hnd = zend_get_std_object_handlers(); +#if PHP_VERSION_ID 503099 + std_hnd-write_property(object, member, value TSRMLS_CC); +#else std_hnd-write_property(object, member, value, key TSRMLS_CC); +#endif } if (member == tmp_member) { @@ -1875,9 +1898,13 @@ } /* }}} */ -/* {{{ php_snmp_has_property(zval *object, zval *member, int has_set_exists) +/* {{{ php_snmp_has_property(zval *object, zval *member, int has_set_exists[, const zend_literal *key]) Generic object property checker */ +#if PHP_VERSION_ID 503099 +static int php_snmp_has_property(zval *object, zval *member, int has_set_exists TSRMLS_DC) +#else static int php_snmp_has_property(zval *object, zval *member, int has_set_exists, const zend_literal *key TSRMLS_DC) +#endif { php_snmp_object *obj = (php_snmp_object *)zend_objects_get_address(object TSRMLS_CC); php_snmp_prop_handler *hnd; @@ -1889,7 +1916,11 @@ ret = 1; break; case 0: { +#if PHP_VERSION_ID 503099 + zval *value = php_snmp_read_property(object, member, BP_VAR_IS TSRMLS_CC); +#else zval *value = php_snmp_read_property(object, member, BP_VAR_IS, key TSRMLS_CC); +#endif if (value != EG(uninitialized_zval_ptr)) { ret = Z_TYPE_P(value) != IS_NULL? 1:0; /* refcount is 0 */ @@ -1899,7 +1930,11 @@
Re: [PHP-CVS] svn: /php/php-src/trunk/ext/snmp/ snmp.c
Hi. I use this module in my production Cacti environment, which uses 5.2 branch. As I use FreeBSD, I'm able to build snmp extension separately from base php as .so module and I use this possibility :) As long as Cacti would need 5.2 to run I will mantain those ugly checks (I believe this legacy will be corrected in next Cacti release). That is the only reason to make ext/snmp module be backwards compatible down-to php5.2 On Tue, Feb 1, 2011 at 11:34 AM, Pierre Joye pierre@gmail.com wrote: hi, I wonder why you need that as we don't release snmp outside php. I would suggest to either drop these backward compatible checks (cleaner code) or consider to do release through pecl as well and then these checks make sense. Cheers, On Tue, Feb 1, 2011 at 9:20 AM, Boris Lytochkin lytbo...@php.net wrote: lytboris Tue, 01 Feb 2011 08:20:13 + Revision: http://svn.php.net/viewvc?view=revisionrevision=307898 Log: preprocessed changes made in rev.307894: keeping ext/snmp backwards compatible Changed paths: U php/php-src/trunk/ext/snmp/snmp.c -- Boris -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-CVS] svn: /php/php-src/trunk/ext/snmp/ snmp.c
yes, but this branch is trunk and is not supposed to be 5.2 compatible. If you want to get it out with previous versions support, then go through pecl as well. But without such external releases it will just clutter the codes. Cheers, On Tue, Feb 1, 2011 at 9:51 AM, Lytochkin Boris lytbo...@gmail.com wrote: Hi. I use this module in my production Cacti environment, which uses 5.2 branch. As I use FreeBSD, I'm able to build snmp extension separately from base php as .so module and I use this possibility :) As long as Cacti would need 5.2 to run I will mantain those ugly checks (I believe this legacy will be corrected in next Cacti release). That is the only reason to make ext/snmp module be backwards compatible down-to php5.2 On Tue, Feb 1, 2011 at 11:34 AM, Pierre Joye pierre@gmail.com wrote: hi, I wonder why you need that as we don't release snmp outside php. I would suggest to either drop these backward compatible checks (cleaner code) or consider to do release through pecl as well and then these checks make sense. Cheers, On Tue, Feb 1, 2011 at 9:20 AM, Boris Lytochkin lytbo...@php.net wrote: lytboris Tue, 01 Feb 2011 08:20:13 + Revision: http://svn.php.net/viewvc?view=revisionrevision=307898 Log: preprocessed changes made in rev.307894: keeping ext/snmp backwards compatible Changed paths: U php/php-src/trunk/ext/snmp/snmp.c -- Boris -- Pierre @pierrejoye | http://blog.thepimp.net | http://www.libgd.org -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php