Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:45349
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain prohost.org from 74.125.78.26 cause and error)
Cc: 'PHP Internals' <internals@lists.php.net>
Message-ID: <9E8BD3A3-8915-4492-88A9-7EA1A0CF6D86@prohost.org>
To: Stanislav Malyshev <stas@zend.com>
In-Reply-To: <4A92D936.2010107@zend.com>
Content-Type: text/plain; charset=US-ASCII; format=flowed; delsp=yes
Content-Transfer-Encoding: 7bit
Mime-Version: 1.0 (Apple Message framework v936)
Date: Mon, 24 Aug 2009 14:22:04 -0400
References: <4A92D936.2010107@zend.com>
Subject: Re: [PHP-DEV] [patch] error masks
From: ilia@prohost.org (Ilia Alshanetsky)

Stas,

While the patch is certainly interesting, I think masking errors is a  
really really bad idea.


On 24-Aug-09, at 2:17 PM, Stanislav Malyshev wrote:

> Hi!
>
> I've implemented a patch that allows to "mask" certain types of  
> errors - i.e. make PHP engine completely ignore them. Now even if  
> the error is not reported, it passes full cycle through message  
> string creation, all allocations on the way, etc. even though  
> ultimately the result of it will be thrown out. So I offer a way to  
> make unwanted errors just disappear.
> The patch uses new directive - "error_mask", and reuses  
> orig_error_reporting global (which is not used anywhere in PHP  
> source so I wonder why it exists at all, but that's certainly  
> convenient :) which makes it binary-compatible and fit for 5.3. Of  
> course, in HEAD we could rename it.
> If the value in not 0, that's the mask which would filter out all  
> errors that do not fit the mask (i.e. you could say in production  
> that everything but warnings and errors should be discarded), and if  
> the value is 0 then the mask is the same as error_reporting - i.e.  
> it can be more dynamic and account for @, etc. and all non-reported  
> errors will be discarded. Fatal errors will never be discarded.
> Alternative approach may be to just discard all non-reported errors,  
> but that could be a problem with user error handlers and extension- 
> supplied error callbacks, so the proposed approach is more flexible  
> as you could control discarding and reporting separately.
>
> So, what do you think?
> -- 
> Stanislav Malyshev, Zend Software Architect
> stas@zend.com   http://www.zend.com/
> (408)253-8829   MSN: stas@zend.com
> Index: main/main.c
> ===================================================================
> --- main/main.c	(revision 287183)
> +++ main/main.c	(working copy)
> @@ -640,6 +640,10 @@
> 	char *message;
> 	int is_function = 0;
>
> +	if(!ZEND_CAN_REPORT(type)) {
> +		return;
> +	}
> +
> 	/* get error text into buffer and escape for html if necessary */
> 	buffer_len = vspprintf(&buffer, 0, format, args);
> 	if (PG(html_errors)) {
> @@ -827,6 +831,9 @@
> 	char *params;
> 	va_list args;
>
> +	if(!ZEND_CAN_REPORT(type)) {
> +		return;
> +	}
> 	spprintf(&params, 0, "%s,%s", param1, param2);
> 	va_start(args, format);
> 	php_verror(docref, params ? params : "...", type, format, args  
> TSRMLS_CC);
> Index: Zend/zend.c
> ===================================================================
> --- Zend/zend.c	(revision 287462)
> +++ Zend/zend.c	(working copy)
> @@ -76,6 +76,17 @@
> }
> /* }}} */
>
> +static ZEND_INI_MH(OnUpdateErrorMask) /* {{{ */
> +{
> +	if (!new_value) {
> +		EG(orig_error_reporting) = EG(error_reporting);
> +	} else {
> +		EG(orig_error_reporting) = atoi(new_value);
> +	}
> +	return SUCCESS;
> +}
> +/* }}} */
> +
> static ZEND_INI_MH(OnUpdateGCEnabled) /* {{{ */
> {
> 	OnUpdateBool(entry, new_value, new_value_length, mh_arg1, mh_arg2,  
> mh_arg3, stage TSRMLS_CC);
> @@ -90,6 +101,7 @@
>
> ZEND_INI_BEGIN()
> 	ZEND_INI_ENTRY("error_reporting",				NULL,		ZEND_INI_ALL,		 
> OnUpdateErrorReporting)
> +	ZEND_INI_ENTRY("error_mask",					"-1",		ZEND_INI_ALL,		 
> OnUpdateErrorMask)
> 	STD_ZEND_INI_BOOLEAN("zend.enable_gc",				"1",	ZEND_INI_ALL,		 
> OnUpdateGCEnabled,      gc_enabled,     zend_gc_globals,         
> gc_globals)
> #ifdef ZEND_MULTIBYTE
>  	STD_ZEND_INI_BOOLEAN("detect_unicode", "1", ZEND_INI_ALL,  
> OnUpdateBool, detect_unicode, zend_compiler_globals, compiler_globals)
> @@ -971,6 +983,10 @@
> 	zend_class_entry *saved_class_entry;
> 	TSRMLS_FETCH();
>
> +	if(!ZEND_CAN_REPORT(type)) {
> +		return;
> +	}
> +	
> 	/* Obtain relevant filename and lineno */
> 	switch (type) {
> 		case E_CORE_ERROR:
> Index: Zend/zend.h
> ===================================================================
> --- Zend/zend.h	(revision 287462)
> +++ Zend/zend.h	(working copy)
> @@ -766,6 +766,13 @@
> ZEND_API void zend_replace_error_handling(zend_error_handling_t  
> error_handling, zend_class_entry *exception_class,  
> zend_error_handling *current TSRMLS_DC);
> ZEND_API void zend_restore_error_handling(zend_error_handling *saved  
> TSRMLS_DC);
>
> +#define ZEND_FATAL_ERROR_MASK (E_CORE_ERROR|E_PARSE|E_COMPILE_ERROR| 
> E_ERROR|E_USER_ERROR)
> +#define ZEND_CAN_REPORT(type)  (\
> +	((type & ZEND_FATAL_ERROR_MASK) != 0) || \
> +	(EG(orig_error_reporting) == 0 && (type & EG(error_reporting)) !=  
> 0) || \
> +	(type & EG(orig_error_reporting)) != 0 \
> +)	
> +
> #endif /* ZEND_H */
>
> /*
>
> -- 
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php