Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:51081
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain sugarcrm.com designates 67.192.241.153 as permitted sender)
Message-ID: <4D0D7CA1.8030200@sugarcrm.com>
Date: Sat, 18 Dec 2010 19:31:45 -0800
Organization: SugarCRM
User-Agent: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2.13) Gecko/20101207 Thunderbird/3.1.7
MIME-Version: 1.0
To: PHP Internals <internals@lists.php.net>
Content-Type: multipart/mixed;
 boundary="------------010908060307020306010403"
Subject: [path] fixing errors, take 2
From: smalyshev@sugarcrm.com (Stas Malyshev)

--------------010908060307020306010403
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

Hi!

Since my first attempt to make error reporting in PHP more efficient 
(see epic thread here: 
http://www.pubbs.net/200908/php/49633-php-dev-patch-error-masks.html) I 
thought about another approach to fixing it.
This approach eliminates the need for additional .ini setting and 
shortcuts only those errors that would not have any consequences - i.e. 
would not be displayed, logged, stored, converted to exception or 
otherwise have any effect on the outside world. The benefit of this 
approach is obvious - nothing changes for the user, only the code runs 
faster. The cost is that we add one handler to utility_functions and 
thus code that overrides error callback (debuggers, etc.) would have to 
provide their own handler if they want the same functionality (by 
default if this handler detects somebody stole error callback it turns off).
The patch is attached.
Any objections for trunk?
-- 
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227

--------------010908060307020306010403
Content-Type: text/plain; x-mac-type="0"; x-mac-creator="0";
 name="errors.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="errors.diff"

Index: Zend/zend.c
===================================================================
--- Zend/zend.c	(revision 306256)
+++ Zend/zend.c	(working copy)
@@ -60,6 +60,7 @@
 int (*zend_vspprintf)(char **pbuf, size_t max_len, const char *format, va_list ap);
 ZEND_API char *(*zend_getenv)(char *name, size_t name_len TSRMLS_DC);
 ZEND_API char *(*zend_resolve_path)(const char *filename, int filename_len TSRMLS_DC);
+ZEND_API int (*zend_suppress_error_cb)(int type TSRMLS_DC);
 
 void (*zend_on_timeout)(int seconds TSRMLS_DC);
 
@@ -649,6 +650,7 @@
 	zend_vspprintf = utility_functions->vspprintf_function;
 	zend_getenv = utility_functions->getenv_function;
 	zend_resolve_path = utility_functions->resolve_path_function;
+	zend_suppress_error_cb = utility_functions->suppress_error;
 
 #if HAVE_DTRACE
 /* build with dtrace support */
@@ -1013,6 +1015,15 @@
 	}
 #endif /* HAVE_DTRACE */
 
+	if (!EG(user_error_handler)
+		|| !(EG(user_error_handler_error_reporting) & type)
+		|| EG(error_handling) != EH_NORMAL) {
+		if(zend_suppress_error_cb(type TSRMLS_CC)) {
+			/* this error goes nowhere, don't bother */
+			va_end(args);
+			return;
+		}	
+	}
 	/* if we don't have a user defined error handler */
 	if (!EG(user_error_handler)
 		|| !(EG(user_error_handler_error_reporting) & type)
Index: Zend/zend.h
===================================================================
--- Zend/zend.h	(revision 306256)
+++ Zend/zend.h	(working copy)
@@ -540,6 +540,7 @@
 	int (*vspprintf_function)(char **pbuf, size_t max_len, const char *format, va_list ap);
 	char *(*getenv_function)(char *name, size_t name_len TSRMLS_DC);
 	char *(*resolve_path_function)(const char *filename, int filename_len TSRMLS_DC);
+	int (*suppress_error)(int type TSRMLS_DC);
 } zend_utility_functions;
 
 typedef struct _zend_utility_values {
Index: main/main.c
===================================================================
--- main/main.c	(revision 306256)
+++ main/main.c	(working copy)
@@ -104,6 +104,7 @@
 /* }}} */
 
 PHPAPI int (*php_register_internal_extensions_func)(TSRMLS_D) = php_register_internal_extensions;
+static void php_error_cb(int type, const char *error_filename, const uint error_lineno, const char *format, va_list args);
 
 #ifndef ZTS
 php_core_globals core_globals;
@@ -602,6 +603,66 @@
 }
 /* }}} */
 
+/* {{ php_suppress_error_cb
+   checks if certain error type should be ignored because it is not going to be reported
+*/
+static int php_suppress_error_cb(int type TSRMLS_DC)
+{
+	if(zend_error_cb != php_error_cb) {
+		/* somebody stole our error handler - he better use his own suppressor */
+		return 0;
+	}
+	switch (type) {
+		/* too serious to ever ignore */
+		case E_CORE_ERROR:
+		case E_ERROR:
+		case E_RECOVERABLE_ERROR:
+		case E_PARSE:
+		case E_COMPILE_ERROR:
+		case E_USER_ERROR:
+				return 0;
+	}
+	
+	if (PG(ignore_repeated_errors)) {
+		/* to ignore repeated errors we need to know error message - can't suppress */
+		return 0;
+	}
+	
+	if (EG(error_handling) != EH_NORMAL) {
+		/* abnormal handling isn't suppressed, unless it's really normal handling in disguise */
+		switch (type) {
+			case E_ERROR:
+			case E_CORE_ERROR:
+			case E_COMPILE_ERROR:
+			case E_USER_ERROR:
+			case E_PARSE:
+				/* fatal errors are real errors and cannot be made exceptions */
+				break;
+			case E_STRICT:
+			case E_DEPRECATED:
+			case E_USER_DEPRECATED:
+				/* for the sake of BC to old damaged code */
+				break;
+			case E_NOTICE:
+			case E_USER_NOTICE:
+				/* notices are no errors and are not treated as such like E_WARNINGS */
+				break;
+			default:
+				return 0;
+		}
+	}
+	if ((EG(error_reporting) & type || (type & E_CORE)) 
+		&& (PG(log_errors) || PG(display_errors) || (!module_initialized))) {
+			return 0;
+	}
+	if (PG(track_errors) && module_initialized) {
+		return 0;
+	}
+
+	return 1;
+}
+/* }} */
+
 /* {{{ php_verror */
 /* php_verror is called from php_error_docref<n> functions.
  * Its purpose is to unify error messages and automatically generate clickable
@@ -622,6 +683,11 @@
 	char *message;
 	int is_function = 0;
 
+	if(php_suppress_error_cb(type TSRMLS_CC)) {
+		/* this error goes nowhere, don't bother */
+		return;
+	}
+
 	/* get error text into buffer and escape for html if necessary */
 	buffer_len = vspprintf(&buffer, 0, format, args);
 
@@ -856,6 +922,10 @@
 	int buffer_len, display;
 	TSRMLS_FETCH();
 
+	if (php_suppress_error_cb(type TSRMLS_CC)) {
+		return;
+	}
+
 	buffer_len = vspprintf(&buffer, PG(log_errors_max_len), format, args);
 
 	/* check for repeated errors to be ignored */
@@ -1886,6 +1956,7 @@
 	zuf.vspprintf_function = vspprintf;
 	zuf.getenv_function = sapi_getenv;
 	zuf.resolve_path_function = php_resolve_path_for_zend;
+	zuf.suppress_error = php_suppress_error_cb;
 	zend_startup(&zuf, NULL TSRMLS_CC);
 
 #ifdef ZTS

--------------010908060307020306010403--