ID: 40392 User updated by: tuxracer69 at gmail dot com Reported By: tuxracer69 at gmail dot com Status: Open Bug Type: Unknown/Other Function Operating System: Linux 2.6.17-2-686 PHP Version: 5CVS-2007-02-07 (snap) New Comment:
oops the exit_status was broken, should be: diff -u php_milter.c.dist php_milter.c --- php_milter.c.dist 2007-02-07 10:18:03.000000000 +0000 +++ php_milter.c 2007-02-09 08:48:03.000000000 +0000 @@ -63,6 +63,8 @@ #include "libmilter/mfapi.h" +#include "php_getopt.h" + #define OPTSTRING "ac:d:Def:hnp:vVz:?" #define MG(v) TSRMG(milter_globals_id, zend_milter_globals *, v) @@ -202,7 +204,7 @@ call_user_function(CG(function_table), NULL, &function_name, &retval, 1, param TSRMLS_CC); MG(state) = MLFI_NONE; - + zval_ptr_dtor(param); if (Z_TYPE(retval) == IS_LONG) { return Z_LVAL(retval); } @@ -235,8 +237,8 @@ call_user_function(CG(function_table), NULL, &function_name, &retval, 1, param TSRMLS_CC); MG(state) = MLFI_NONE; + zval_ptr_dtor(param); - FREE_ZVAL(param[0]); if (Z_TYPE(retval) == IS_LONG) { return Z_LVAL(retval); @@ -275,8 +277,8 @@ call_user_function(CG(function_table), NULL, &function_name, &retval, 1, param TSRMLS_CC); MG(state) = MLFI_NONE; + zval_ptr_dtor(param); - FREE_ZVAL(param[0]); if (Z_TYPE(retval) == IS_LONG) { return Z_LVAL(retval); @@ -316,7 +318,8 @@ MG(state) = MLFI_NONE; - FREE_ZVAL(param[0]); + zval_ptr_dtor(param); + if (Z_TYPE(retval) == IS_LONG) { return Z_LVAL(retval); @@ -354,8 +357,9 @@ MG(state) = MLFI_NONE; - FREE_ZVAL(param[0]); - FREE_ZVAL(param[1]); + zval_ptr_dtor(¶m[0]); + zval_ptr_dtor(¶m[1]); + if (Z_TYPE(retval) == IS_LONG) { return Z_LVAL(retval); @@ -408,7 +412,7 @@ INIT_PZVAL(param[0]); ZVAL_STRING(&function_name, "milter_body", 0); - ZVAL_STRINGL(param[0], bodyp, len, 1); + ZVAL_STRINGL(param[0], (char*)bodyp, len, 1); /*alex*/ /* set the milter context for possible use in API functions */ MG(ctx) = ctx; @@ -418,7 +422,8 @@ MG(state) = MLFI_NONE; - FREE_ZVAL(param[0]); + zval_ptr_dtor(param); + if (Z_TYPE(retval) == IS_LONG) { return Z_LVAL(retval); @@ -548,7 +553,7 @@ if (MG(state) != MLFI_INIT) { php_error(E_WARNING, NOT_INIT, get_active_function_name(TSRMLS_C)); } else if (zend_parse_parameters(1 TSRMLS_CC, "l", &flags) == SUCCESS) { - flags = flags & SMFIF_ADDHDRS|SMFIF_CHGHDRS|SMFIF_CHGBODY|SMFIF_ADDRCPT|SMFIF_DELRCPT; + flags = flags & (SMFIF_ADDHDRS|SMFIF_CHGHDRS|SMFIF_CHGBODY|SMFIF_ADDRCPT|SMFIF_DELRCPT); smfilter.xxfi_flags = flags; } } @@ -703,7 +708,7 @@ if (MG(state) != MLFI_EOM) { php_error(E_WARNING, NOT_EOM, get_active_function_name(TSRMLS_C)); } else if (zend_parse_parameters(1 TSRMLS_CC, "s", &body, &len) == SUCCESS) { - if (smfi_replacebody(MG(ctx), body, len) == MI_SUCCESS) { + if (smfi_replacebody(MG(ctx), (u_char*)body, len) == MI_SUCCESS) { RETURN_TRUE; } } @@ -732,6 +737,8 @@ MG(state) = MLFI_NONE; MG(initialized) = 0; + return SUCCESS; + } /* }}} */ @@ -920,9 +927,7 @@ /* temporary locals */ int orig_optind=ap_php_optind; char *orig_optarg=ap_php_optarg; - char *arg_free=NULL, **arg_excp=&arg_free; int interactive=0; - char *exec_direct=NULL; char *param_error=NULL; /* end of temporary locals */ @@ -1099,7 +1104,7 @@ openlog("php-milter", LOG_PID, LOG_MAIL); - if (exit_status = mlfi_init()) { + if ((exit_status = mlfi_init())) { syslog(1, "mlfi_init failed."); exit(exit_status); } Previous Comments: ------------------------------------------------------------------------ [2007-02-09 08:10:42] tuxracer69 at gmail dot com a longer patch aimed at fixing besides the mem leaks the compile warnings (type cast, declarations, parenthesis). diff -u php_milter.c.dist php_milter.c --- php_milter.c.dist 2007-02-07 10:18:03.000000000 +0000 +++ php_milter.c 2007-02-09 08:01:01.000000000 +0000 @@ -63,6 +63,8 @@ #include "libmilter/mfapi.h" +#include "php_getopt.h" + #define OPTSTRING "ac:d:Def:hnp:vVz:?" #define MG(v) TSRMG(milter_globals_id, zend_milter_globals *, v) @@ -202,7 +204,7 @@ call_user_function(CG(function_table), NULL, &function_name, &retval, 1, param TSRMLS_CC); MG(state) = MLFI_NONE; - + zval_ptr_dtor(param); if (Z_TYPE(retval) == IS_LONG) { return Z_LVAL(retval); } @@ -235,8 +237,8 @@ call_user_function(CG(function_table), NULL, &function_name, &retval, 1, param TSRMLS_CC); MG(state) = MLFI_NONE; + zval_ptr_dtor(param); - FREE_ZVAL(param[0]); if (Z_TYPE(retval) == IS_LONG) { return Z_LVAL(retval); @@ -275,8 +277,8 @@ call_user_function(CG(function_table), NULL, &function_name, &retval, 1, param TSRMLS_CC); MG(state) = MLFI_NONE; + zval_ptr_dtor(param); - FREE_ZVAL(param[0]); if (Z_TYPE(retval) == IS_LONG) { return Z_LVAL(retval); @@ -316,7 +318,8 @@ MG(state) = MLFI_NONE; - FREE_ZVAL(param[0]); + zval_ptr_dtor(param); + if (Z_TYPE(retval) == IS_LONG) { return Z_LVAL(retval); @@ -354,8 +357,9 @@ MG(state) = MLFI_NONE; - FREE_ZVAL(param[0]); - FREE_ZVAL(param[1]); + zval_ptr_dtor(¶m[0]); + zval_ptr_dtor(¶m[1]); + if (Z_TYPE(retval) == IS_LONG) { return Z_LVAL(retval); @@ -408,7 +412,7 @@ INIT_PZVAL(param[0]); ZVAL_STRING(&function_name, "milter_body", 0); - ZVAL_STRINGL(param[0], bodyp, len, 1); + ZVAL_STRINGL(param[0], (char*)bodyp, len, 1); /*alex*/ /* set the milter context for possible use in API functions */ MG(ctx) = ctx; @@ -418,7 +422,8 @@ MG(state) = MLFI_NONE; - FREE_ZVAL(param[0]); + zval_ptr_dtor(param); + if (Z_TYPE(retval) == IS_LONG) { return Z_LVAL(retval); @@ -548,7 +553,7 @@ if (MG(state) != MLFI_INIT) { php_error(E_WARNING, NOT_INIT, get_active_function_name(TSRMLS_C)); } else if (zend_parse_parameters(1 TSRMLS_CC, "l", &flags) == SUCCESS) { - flags = flags & SMFIF_ADDHDRS|SMFIF_CHGHDRS|SMFIF_CHGBODY|SMFIF_ADDRCPT|SMFIF_DELRCPT; + flags = flags & (SMFIF_ADDHDRS|SMFIF_CHGHDRS|SMFIF_CHGBODY|SMFIF_ADDRCPT|SMFIF_DELRCPT); smfilter.xxfi_flags = flags; } } @@ -703,7 +708,7 @@ if (MG(state) != MLFI_EOM) { php_error(E_WARNING, NOT_EOM, get_active_function_name(TSRMLS_C)); } else if (zend_parse_parameters(1 TSRMLS_CC, "s", &body, &len) == SUCCESS) { - if (smfi_replacebody(MG(ctx), body, len) == MI_SUCCESS) { + if (smfi_replacebody(MG(ctx), (u_char*)body, len) == MI_SUCCESS) { RETURN_TRUE; } } @@ -732,6 +737,7 @@ MG(state) = MLFI_NONE; MG(initialized) = 0; + return SUCCESS; } /* }}} */ @@ -920,9 +926,7 @@ /* temporary locals */ int orig_optind=ap_php_optind; char *orig_optarg=ap_php_optarg; - char *arg_free=NULL, **arg_excp=&arg_free; int interactive=0; - char *exec_direct=NULL; char *param_error=NULL; /* end of temporary locals */ @@ -1099,7 +1103,7 @@ openlog("php-milter", LOG_PID, LOG_MAIL); - if (exit_status = mlfi_init()) { + if (exit_status == mlfi_init()) { syslog(1, "mlfi_init failed."); exit(exit_status); } ------------------------------------------------------------------------ [2007-02-09 07:18:05] tuxracer69 at gmail dot com Unified diff for the code change above: diff -u php_milter.c.dist php_milter.c --- php_milter.c.dist 2007-02-07 10:18:03.000000000 +0000 +++ php_milter.c 2007-02-07 19:41:54.000000000 +0000 @@ -202,7 +202,7 @@ call_user_function(CG(function_table), NULL, &function_name, &retval, 1, param TSRMLS_CC); MG(state) = MLFI_NONE; - + zval_ptr_dtor(param); if (Z_TYPE(retval) == IS_LONG) { return Z_LVAL(retval); } @@ -235,8 +235,8 @@ call_user_function(CG(function_table), NULL, &function_name, &retval, 1, param TSRMLS_CC); MG(state) = MLFI_NONE; + zval_ptr_dtor(param); - FREE_ZVAL(param[0]); if (Z_TYPE(retval) == IS_LONG) { return Z_LVAL(retval); @@ -275,8 +275,8 @@ call_user_function(CG(function_table), NULL, &function_name, &retval, 1, param TSRMLS_CC); MG(state) = MLFI_NONE; + zval_ptr_dtor(param); - FREE_ZVAL(param[0]); if (Z_TYPE(retval) == IS_LONG) { return Z_LVAL(retval); @@ -316,7 +316,8 @@ MG(state) = MLFI_NONE; - FREE_ZVAL(param[0]); + zval_ptr_dtor(param); + if (Z_TYPE(retval) == IS_LONG) { return Z_LVAL(retval); @@ -354,8 +355,9 @@ MG(state) = MLFI_NONE; - FREE_ZVAL(param[0]); - FREE_ZVAL(param[1]); + zval_ptr_dtor(¶m[0]); + zval_ptr_dtor(¶m[1]); + if (Z_TYPE(retval) == IS_LONG) { return Z_LVAL(retval); @@ -418,7 +420,8 @@ MG(state) = MLFI_NONE; - FREE_ZVAL(param[0]); + zval_ptr_dtor(param); + if (Z_TYPE(retval) == IS_LONG) { return Z_LVAL(retval); ------------------------------------------------------------------------ [2007-02-07 22:10:56] tuxracer69 at gmail dot com Well, the goal of the sapi is to call PHP code based on SMTP commands a client sends to a sendmail server, and in that respect I would say 'it works'. Although I did not put the code in a production server, (that's what I eventually plan to do) I would say that the functions do what they are expected to do. The memory leaks were really ugly. I have other warnings at compile time I could have a look at. I think there is definitely an interest in processing mail with PHP as after a previous bug we fixed in the milter sapi (Bug #40083) I got mails of other developers asking questions about the stability of the sapi. Perhaps moving this code to PECL would slow down its development at a moment where interest is growing and a (relatively) small effort may take the code up to date. thanks Alex ------------------------------------------------------------------------ [2007-02-07 20:12:48] [EMAIL PROTECTED] >Could somebody confirm this? Yes. This SAPI is orphaned for years, so I would actually like to move it to PECL (or anywhere else, just because we cannot guarantee if it works at all). Also looking at the code I can see other problems, one more reason to remove this SAPI. ------------------------------------------------------------------------ [2007-02-07 20:03:20] tuxracer69 at gmail dot com after digging in the cli SAPI code I tried using zval_ptr_dtor instead of the FREE_ZVAL macro. This resolved the issue (at least I do not see the warnings anymore). Could somebody confirm this? Thanks Alex ----here is my diff--- diff php_milter.c php_milter.c.dist 205c205 < zval_ptr_dtor(param); --- > 238d237 < zval_ptr_dtor(param); 239a239 > FREE_ZVAL(param[0]); 278d277 < zval_ptr_dtor(param); 279a279 > FREE_ZVAL(param[0]); 319,320c319 < zval_ptr_dtor(param); < --- > FREE_ZVAL(param[0]); 358,360c357,358 < zval_ptr_dtor(¶m[0]); < zval_ptr_dtor(¶m[1]); < --- > FREE_ZVAL(param[0]); > FREE_ZVAL(param[1]); 423,424c421 < zval_ptr_dtor(param); < --- > FREE_ZVAL(param[0]); ------------------------------------------------------------------------ The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at http://bugs.php.net/40392 -- Edit this bug report at http://bugs.php.net/?id=40392&edit=1