Another pass making some failure states unconditional erros in PDO

Also make internal function return type more accurate to inform usage

ext/pdo/pdo_dbh.c

@@ -34,7 +34,7 @@
 #include "zend_interfaces.h"
 #include "pdo_dbh_arginfo.h"
 
-static int pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value);
+static zend_result pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value);
 
 void pdo_throw_exception(unsigned int driver_errcode, char *driver_errmsg, pdo_error_type *pdo_error)
 {
@@ -405,6 +405,7 @@ options:
 					continue;
 				}
 				ZVAL_DEREF(attr_value);
+				/* TODO: Check that this doesn't fail? */
 				pdo_dbh_attribute_set(dbh, long_key, attr_value);
 			} ZEND_HASH_FOREACH_END();
 		}
@@ -434,6 +435,9 @@ static zval *pdo_stmt_instantiate(pdo_dbh_t *dbh, zval *object, zend_class_entry
 	}
 
 	if (UNEXPECTED(object_init_ex(object, dbstmt_ce) != SUCCESS)) {
+		if (EXPECTED(!EG(exception))) {
+			zend_throw_error(NULL, "Cannot instantiate user-supplied statement class");
+		}
 		return NULL;
 	}
 
@@ -544,13 +548,8 @@ PHP_METHOD(PDO, prepare)
 		ZVAL_COPY_VALUE(&ctor_args, &dbh->def_stmt_ctor_args);
 	}
 
-	/* Need to check if pdo_stmt_instantiate() throws an exception unconditionally to see if can change the RETURN_FALSE; */
 	if (!pdo_stmt_instantiate(dbh, return_value, dbstmt_ce, &ctor_args)) {
-		if (EXPECTED(!EG(exception))) {
-			zend_throw_error(NULL, "Cannot instantiate user-supplied statement class");
-		}
-		PDO_HANDLE_DBH_ERR();
-		RETURN_FALSE;
+		RETURN_THROWS();
 	}
 	stmt = Z_PDO_STMT_P(return_value);
 
@@ -674,7 +673,7 @@ PHP_METHOD(PDO, inTransaction)
 }
 /* }}} */
 
-static int pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /* {{{ */
+static zend_result pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /* {{{ */
 {
 	zend_long lval;
 
@@ -752,6 +751,7 @@ static int pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /*
 			zval *item;
 
 			if (dbh->is_persistent) {
+				/* TODO: ValueError/ PDOException? */
 				pdo_raise_impl_error(dbh, NULL, "HY000",
 					"PDO::ATTR_STATEMENT_CLASS cannot be used with persistent PDO instances"
 					);
@@ -1070,10 +1070,7 @@ PHP_METHOD(PDO, query)
 	PDO_DBH_CLEAR_ERR();
 
 	if (!pdo_stmt_instantiate(dbh, return_value, dbh->def_stmt_ce, &dbh->def_stmt_ctor_args)) {
-		if (EXPECTED(!EG(exception))) {
-			pdo_raise_impl_error(dbh, NULL, "HY000", "failed to instantiate user supplied statement class");
-		}
-		return;
+		RETURN_THROWS();
 	}
 	stmt = Z_PDO_STMT_P(return_value);
 

ext/pdo/pdo_stmt.c

@@ -41,7 +41,7 @@
 		RETURN_THROWS(); \
 	} \
 
-static inline int rewrite_name_to_position(pdo_stmt_t *stmt, struct pdo_bound_param_data *param) /* {{{ */
+static inline bool rewrite_name_to_position(pdo_stmt_t *stmt, struct pdo_bound_param_data *param) /* {{{ */
 {
 	if (stmt->bound_param_map) {
 		/* rewriting :name to ? style.
@@ -90,9 +90,9 @@ static inline int rewrite_name_to_position(pdo_stmt_t *stmt, struct pdo_bound_pa
 /* }}} */
 
 /* trigger callback hook for parameters */
-static int dispatch_param_event(pdo_stmt_t *stmt, enum pdo_param_event event_type) /* {{{ */
+static bool dispatch_param_event(pdo_stmt_t *stmt, enum pdo_param_event event_type) /* {{{ */
 {
-	int ret = 1, is_param = 1;
+	bool ret = 1, is_param = 1;
 	struct pdo_bound_param_data *param;
 	HashTable *ht;
 
@@ -212,7 +212,7 @@ static void param_dtor(zval *el) /* {{{ */
 }
 /* }}} */
 
-static int really_register_bound_param(struct pdo_bound_param_data *param, pdo_stmt_t *stmt, int is_param) /* {{{ */
+static bool really_register_bound_param(struct pdo_bound_param_data *param, pdo_stmt_t *stmt, bool is_param) /* {{{ */
 {
 	HashTable *hash;
 	zval *parameter;
@@ -381,12 +381,7 @@ PHP_METHOD(PDOStatement, execute)
 				param.paramno = -1;
 			} else {
 				/* we're okay to be zero based here */
-				/* num_index is unsignend
-				if (num_index < 0) {
-					pdo_raise_impl_error(stmt->dbh, stmt, "HY093", NULL);
-					RETURN_FALSE;
-				}
-				*/
+				/* num_index is unsignend */
 				param.paramno = num_index;
 			}
 
@@ -601,7 +596,7 @@ static inline void fetch_value(pdo_stmt_t *stmt, zval *dest, int colno, int *typ
 }
 /* }}} */
 
-static int do_fetch_common(pdo_stmt_t *stmt, enum pdo_fetch_orientation ori, zend_long offset) /* {{{ */
+static bool do_fetch_common(pdo_stmt_t *stmt, enum pdo_fetch_orientation ori, zend_long offset) /* {{{ */
 {
 	if (!stmt->executed) {
 		return 0;
@@ -652,7 +647,7 @@ static int do_fetch_common(pdo_stmt_t *stmt, enum pdo_fetch_orientation ori, zen
 }
 /* }}} */
 
-static int do_fetch_class_prepare(pdo_stmt_t *stmt) /* {{{ */
+static bool do_fetch_class_prepare(pdo_stmt_t *stmt) /* {{{ */
 {
 	zend_class_entry *ce = stmt->fetch.cls.ce;
 	zend_fcall_info *fci = &stmt->fetch.cls.fci;
@@ -677,8 +672,7 @@ static int do_fetch_class_prepare(pdo_stmt_t *stmt) /* {{{ */
 		fcc->called_scope = ce;
 		return 1;
 	} else if (!Z_ISUNDEF(stmt->fetch.cls.ctor_args)) {
-		/* TODO Error? */
-		pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "user-supplied class does not have a constructor, use NULL for the ctor_params parameter, or simply omit it");
+		zend_throw_error(NULL, "User-supplied statement does not accept constructor arguments");
 		return 0;
 	} else {
 		return 1; /* no ctor no args is also ok */
@@ -686,7 +680,7 @@ static int do_fetch_class_prepare(pdo_stmt_t *stmt) /* {{{ */
 }
 /* }}} */
 
-static int make_callable_ex(pdo_stmt_t *stmt, zval *callable, zend_fcall_info * fci, zend_fcall_info_cache * fcc, int num_args) /* {{{ */
+static bool make_callable_ex(pdo_stmt_t *stmt, zval *callable, zend_fcall_info * fci, zend_fcall_info_cache * fcc, int num_args) /* {{{ */
 {
 	char *is_callable_error = NULL;
 
@@ -809,6 +803,7 @@ static bool do_fetch(pdo_stmt_t *stmt, zval *return_value, enum pdo_fetch_type h
 
 		case PDO_FETCH_KEY_PAIR:
 			if (stmt->column_count != 2) {
+				/* TODO: Error? */
 				pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "PDO::FETCH_KEY_PAIR fetch mode requires the result set to contain exactly 2 columns.");
 				return 0;
 			}
@@ -904,6 +899,7 @@ static bool do_fetch(pdo_stmt_t *stmt, zval *return_value, enum pdo_fetch_type h
 			break;
 
 		case PDO_FETCH_INTO:
+			/* TODO: Make this an assertion and ensure this is true higher up? */
 			if (Z_ISUNDEF(stmt->fetch.into)) {
 				/* TODO ArgumentCountError? */
 				pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "No fetch-into object specified.");
@@ -919,6 +915,7 @@ static bool do_fetch(pdo_stmt_t *stmt, zval *return_value, enum pdo_fetch_type h
 			break;
 
 		case PDO_FETCH_FUNC:
+			/* TODO: Make this an assertion and ensure this is true higher up? */
 			if (Z_ISUNDEF(stmt->fetch.func.function)) {
 				/* TODO ArgumentCountError? */
 				pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "No fetch function specified");
@@ -1633,7 +1630,7 @@ PHP_METHOD(PDOStatement, setAttribute)
 
 /* {{{ Get an attribute */
 
-static int generic_stmt_attr_get(pdo_stmt_t *stmt, zval *return_value, zend_long attr)
+static bool generic_stmt_attr_get(pdo_stmt_t *stmt, zval *return_value, zend_long attr)
 {
 	switch (attr) {
 		case PDO_ATTR_EMULATE_PREPARES:

ext/pdo_mysql/tests/pdo_mysql_attr_statement_class.phpt

@@ -114,13 +114,15 @@ $db = MySQLPDOTest::factory();
     // Yes, this is a fatal error and I want it to fail.
     abstract class mystatement6 extends mystatement5 {
     }
-    if (true !== ($tmp = $db->setAttribute(PDO::ATTR_STATEMENT_CLASS, array('mystatement6', array('mystatement6')))))
-        printf("[011] Expecting boolean/true got %s\n", var_export($tmp, true));
-    $stmt = $db->query('SELECT id, label FROM test ORDER BY id ASC LIMIT 1');
+    try {
+        $db->setAttribute(PDO::ATTR_STATEMENT_CLASS, ['mystatement6', ['mystatement6']]);
+        $stmt = $db->query('SELECT id, label FROM test ORDER BY id ASC LIMIT 1');
+    } catch (\Error $e) {
+        echo get_class($e), ': ', $e->getMessage(), \PHP_EOL;
+    }
 
-    print "done!";
 ?>
---EXPECTF--
+--EXPECT--
 array(1) {
   [0]=>
   string(12) "PDOStatement"
@@ -157,9 +159,4 @@ array(1) {
     string(1) "a"
   }
 }
-
-Fatal error: Uncaught Error: Cannot instantiate abstract class mystatement6 in %s:%d
-Stack trace:
-#0 %s(%d): PDO->query('SELECT id, labe...')
-#1 {main}
-  thrown in %s on line %d
+Error: Cannot instantiate abstract class mystatement6