Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:9242 Return-Path: Mailing-List: contact internals-help@lists.php.net; run by ezmlm Delivered-To: mailing list internals@lists.php.net Received: (qmail 43866 invoked by uid 1010); 15 Apr 2004 23:08:52 -0000 Delivered-To: ezmlm-scan-internals@lists.php.net Delivered-To: ezmlm-internals@lists.php.net Received: (qmail 43842 invoked from network); 15 Apr 2004 23:08:52 -0000 Received: from unknown (HELO mirapoint.kettering.edu) (192.138.137.82) by pb1.pair.com with SMTP; 15 Apr 2004 23:08:52 -0000 Received: from coogle.localdomain (pcp09031773pcs.mtmors01.mi.comcast.net [68.61.83.224]) by mirapoint.kettering.edu (MOS 3.4.5-GR) with ESMTP id AND14131 (AUTH via LOGINBEFORESMTP); Thu, 15 Apr 2004 19:08:49 -0400 (EDT) Reply-To: john@coggeshall.org To: Nuno Lopes Cc: PHP Internals In-Reply-To: <00cc01c422e1$46a4adc0$0100a8c0@pc07653> References: <00cc01c422e1$46a4adc0$0100a8c0@pc07653> Content-Type: multipart/mixed; boundary="=-vNq/AwMBqy1ctBlTmdKx" Message-ID: <1082070778.17792.18.camel@coogle.localdomain> Mime-Version: 1.0 X-Mailer: Ximian Evolution 1.4.5 Date: Thu, 15 Apr 2004 19:12:58 -0400 Subject: Re: [PHP-DEV] Exceptions and a real example: Tidy From: john@coggeshall.org (John Coggeshall) --=-vNq/AwMBqy1ctBlTmdKx Content-Type: text/plain Content-Transfer-Encoding: 7bit Attached is a patch which I hope will keep people happy when it comes to specifically the Tidy extension. I'd like some feedback on this before I commit it / throw it away: Changes: - All errors were re-evaluated, and those (such as a bogus config option) were demoted to E_NOTICE or promoted to E_ERROR as necessary - Those errors which are truly E_WARNING will behave as such when called from a procedural context. If called from an object oriented context, they will be represented as exceptions. I also looked at the bugs you reported, Nuno but I couldn't reproduce some of them. In either case, the ones i could reproduced should be fixed. Feedback welcome. John On Thu, 2004-04-15 at 08:00, Nuno Lopes wrote: > Hello, > > I've followed the war, sorry, discussion about exceptions. > Now, let me introduce some problems I've found in Tidy. > > Look at the code: > > //doesn't echo any error, but should! > //should generate a E_WARNING because it can't find the file > $tidy = tidy_parse_string('sdgdsg', 'BogusConfig.file'); > > /*********************************************************/ > > > //throw exception instead of E_WARNING > try { > $tidy = tidy_parse_string('sdgdsg', array('bogusconf' => 'bogusvalue')); > > //because of throwing the exception, this function is never executed > //thus making it complitely unusefull. > echo tidy_config_count($tidy); > } > catch (tidy_exception $e) { > echo $e; > } > > /*********************************************************/ > > > //an exception here. why? a BUG?!?!?! > $tidy = new tidy(); > $tidy->ParseString('test'); > > ?> > > > These are just some examples. I don't hate exceptions, but whe they are > misused... > John said that Tidy should only generate exceptions with OO code, but as you > can see in the above code, it generate exceptions with non-OO code. > > > Nuno -- -=~=--=~=--=~=--=~=--=~=--=~=--=~=--=~=--=~=--=~=--=~=--=~=- John Coggeshall http://www.coggeshall.org/ The PHP Developer's Handbook http://www.php-handbook.com/ -=~=--=~=--=~=--=~=--=~=--=~=--=~=--=~=--=~=--=~=--=~=--=~=- --=-vNq/AwMBqy1ctBlTmdKx Content-Disposition: attachment; filename=tidy_except.patch.txt Content-Type: text/plain; name=tidy_except.patch.txt; charset=UTF-8 Content-Transfer-Encoding: 7bit ? tidy_except.patch.txt Index: tidy.c =================================================================== RCS file: /repository/php-src/ext/tidy/tidy.c,v retrieving revision 1.43 diff -u -r1.43 tidy.c --- tidy.c 14 Apr 2004 19:01:45 -0000 1.43 +++ tidy.c 15 Apr 2004 23:06:23 -0000 @@ -238,7 +238,7 @@ PHP_FE(tidy_error_count, NULL) PHP_FE(tidy_warning_count, NULL) PHP_FE(tidy_access_count, NULL) - PHP_FE(tidy_config_count, NULL) + PHP_FE(tidy_config_count, NULL) PHP_FE(tidy_get_root, NULL) PHP_FE(tidy_get_head, NULL) PHP_FE(tidy_get_html, NULL) @@ -346,7 +346,7 @@ if(TG(inst)) { zend_throw_exception(tidy_ce_exception, msg, 0 TSRMLS_CC); } else { - php_error_docref(NULL TSRMLS_CC, E_ERROR, msg); + php_error_docref(NULL TSRMLS_CC, E_WARNING, msg); } va_end(ap); @@ -361,12 +361,13 @@ opt = tidyGetOptionByName(doc, optname); if (!opt) { - TIDY_THROW("Unknown Tidy Configuration Option '%s'", optname); + php_error_docref(NULL TSRMLS_CC, E_NOTICE, "Unknown Tidy Configuration Option '%s'", optname); return FAILURE; } if (tidyOptIsReadOnly(opt)) { - TIDY_THROW("Attempt to set read-only option '%s'", optname); + + php_error_docref(NULL TSRMLS_CC, E_NOTICE, "Attempting to set read-only option '%s'", optname); return FAILURE; } @@ -409,6 +410,8 @@ TidyBuffer *errbuf; zval *config; + TIDY_SET_CONTEXT; + if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s|zsb", &arg1, &arg1_len, &config, &enc, &enc_len, &use_include_path) == FAILURE) { RETURN_FALSE; } @@ -442,7 +445,7 @@ convert_to_string_ex(&config); TIDY_SAFE_MODE_CHECK(Z_STRVAL_P(config)); if (tidyLoadConfig(doc, Z_STRVAL_P(config)) < 0) { - php_error_docref(NULL TSRMLS_CC, E_WARNING, "Could not load configuration file '%s'", Z_STRVAL_P(config)); + TIDY_THROW("Could not load configuration file '%s'", Z_STRVAL_P(config)); RETVAL_FALSE; } } @@ -858,9 +861,10 @@ zend_hash_move_forward(ht_options)) { if(zend_hash_get_current_key(ht_options, &opt_name, &opt_indx, FALSE) == FAILURE) { - TIDY_THROW("Could not retrieve key from option array"); + zend_error(E_ERROR, "Could not retrieve key from option array"); return FAILURE; } + if(opt_name) { _php_tidy_set_tidy_opt(doc, opt_name, *opt_val TSRMLS_CC); } @@ -934,7 +938,7 @@ { if (INI_BOOL("tidy.clean_output") == TRUE) { if (php_start_ob_buffer_named("ob_tidyhandler", 0, 1 TSRMLS_CC) == FAILURE) { - zend_error(E_NOTICE, "Unable to use Tidy for output buffering."); + zend_error(E_NOTICE, "Failure installing Tidy output buffering."); } } @@ -1014,6 +1018,8 @@ zval *options = NULL; PHPTidyObj *obj; + + TIDY_SET_CONTEXT; if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s|zs", &input, &input_len, &options, &enc, &enc_len) == FAILURE) { RETURN_FALSE; @@ -1069,6 +1075,7 @@ zval *options = NULL; PHPTidyObj *obj; + TIDY_SET_CONTEXT; if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s|zsb", &inputfile, &input_len, &options, &enc, &enc_len, &use_include_path) == FAILURE) { @@ -1090,6 +1097,7 @@ INIT_ZVAL(*return_value); RETVAL_FALSE; } + efree(contents); } /* }}} */ @@ -1113,6 +1121,7 @@ Repair a string using an optionally provided configuration file */ PHP_FUNCTION(tidy_repair_string) { + TIDY_SET_CONTEXT; php_tidy_quick_repair(INTERNAL_FUNCTION_PARAM_PASSTHRU, FALSE); } /* }}} */ @@ -1121,6 +1130,7 @@ Repair a file using an optionally provided configuration file */ PHP_FUNCTION(tidy_repair_file) { + TIDY_SET_CONTEXT; php_tidy_quick_repair(INTERNAL_FUNCTION_PARAM_PASSTHRU, TRUE); } /* }}} */ @@ -1144,6 +1154,8 @@ Get release date (version) for Tidy library */ PHP_FUNCTION(tidy_get_release) { + TIDY_SET_CONTEXT; + if (ZEND_NUM_ARGS()) { WRONG_PARAM_COUNT; } @@ -1281,7 +1293,8 @@ int optname_len; TidyOption opt; TidyOptionType optt; - TIDY_SET_CONTEXT; + + TIDY_SET_CONTEXT; if (object) { if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s", &optname, &optname_len) == FAILURE) { @@ -1296,6 +1309,7 @@ obj = (PHPTidyObj *) zend_object_store_get_object(object TSRMLS_CC); opt = tidyGetOptionByName(obj->ptdoc->doc, optname); + if (!opt) { TIDY_THROW("Unknown Tidy Configuration Option '%s'", optname); RETURN_FALSE; @@ -1320,7 +1334,7 @@ break; default: - TIDY_THROW("Unable to determine type of configuration constant"); + TIDY_THROW("Unable to determine type of configuration option"); break; } @@ -1422,6 +1436,7 @@ Returns a TidyNode Object representing the root of the tidy parse tree */ PHP_FUNCTION(tidy_get_root) { + TIDY_SET_CONTEXT; php_tidy_create_node(INTERNAL_FUNCTION_PARAM_PASSTHRU, is_root_node); } /* }}} */ @@ -1430,6 +1445,7 @@ Returns a TidyNode Object starting from the tag of the tidy parse tree */ PHP_FUNCTION(tidy_get_html) { + TIDY_SET_CONTEXT; php_tidy_create_node(INTERNAL_FUNCTION_PARAM_PASSTHRU, is_html_node); } /* }}} */ @@ -1438,6 +1454,7 @@ Returns a TidyNode Object starting from the tag of the tidy parse tree */ PHP_FUNCTION(tidy_get_head) { + TIDY_SET_CONTEXT; php_tidy_create_node(INTERNAL_FUNCTION_PARAM_PASSTHRU, is_head_node); } /* }}} */ @@ -1446,6 +1463,7 @@ Returns a TidyNode Object starting from the tag of the tidy parse tree */ PHP_FUNCTION(tidy_get_body) { + TIDY_SET_CONTEXT; php_tidy_create_node(INTERNAL_FUNCTION_PARAM_PASSTHRU, is_body_node); } /* }}} */ --=-vNq/AwMBqy1ctBlTmdKx--