Another pass making some failure states unconditional erros in PDO

Also make internal function return type more accurate to inform usage

diff --git a/ext/pdo/pdo_dbh.c b/ext/pdo/pdo_dbh.c
index 0f5eaab2e6e62b54569e7ff44340b633b0da26d4..2e81db2bbf52d29131f9a042609fed959bd416bb 100644 (file)
--- a/ext/pdo/pdo_dbh.c
+++ b/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);
 

diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c
index 0cec5d9955c53ef71cf7785b353722cf5c6a47b7..3222a617f07e9b72c1abf0edcbb24e346e9021f7 100644 (file)
--- a/ext/pdo/pdo_stmt.c
+++ b/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:

diff --git a/ext/pdo_mysql/tests/pdo_mysql_attr_statement_class.phpt b/ext/pdo_mysql/tests/pdo_mysql_attr_statement_class.phpt
index 6d536f9deddd215fc0a7fd0275d8b3af752267cb..77f231d28fa653d6a53faeb67dc1ea96e25ce0d7 100644 (file)
--- a/ext/pdo_mysql/tests/pdo_mysql_attr_statement_class.phpt
+++ b/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