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 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
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 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 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 wrote:
>>> lytboris Sat, 26 Feb 2011 19:08:55 +
>>>
>>> Revision: http://svn.php.net/viewvc?view=revision&revision=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
- 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=revision&revision=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
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 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 wrote:
>> lytboris Sat, 26 Feb 2011 19:08:55 +
>>
>> Revision: http://svn.php.net/viewvc?view=revision&revision=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
--
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 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
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 wrote: > lytboris Sat, 26 Feb 2011 19:08:55 + > > Revision: http://svn.php.net/viewvc?view=revision&revision=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
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 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 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 wrote: >>> lytboris Tue, 01 Feb 2011 08:20:13 + >>> >>> Revision: http://svn.php.net/viewvc?view=revision&revision=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
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 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 wrote: >> lytboris Tue, 01 Feb 2011 08:20:13 + >> >> Revision: http://svn.php.net/viewvc?view=revision&revision=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
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 wrote:
> lytboris Tue, 01 Feb 2011 08:20:13 +
>
> Revision: http://svn.php.net/viewvc?view=revision&revision=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)) {
> r
