mysql related test cases are failing on big endian system

[guirish]

mysql related test cases were failing on big endian system, updated type casting during pointer deferencing.

Failing Test cases included:
mysqli_get_client_stats() [ext/mysqli/tests/mysqli_get_client_stats.phpt]
mysqli_stmt_attr_set() - mysqlnd does not check for invalid codes [ext/mysqli/tests/mysqli_stmt_attr_set.phpt]
MySQL PDO->__construct(), options [ext/pdo_mysql/tests/pdo_mysql___construct_options.phpt]
MySQL PDO->exec(), affected rows [ext/pdo_mysql/tests/pdo_mysql_exec_load_data.phpt]
enable local infile [ext/pdo_mysql/tests/pdo_mysql_local_infile_set_on.phpt]

[nikic]

Per https://dev.mysql.com/doc/refman/8.0/en/mysql-options.html the type for MYSQL_OPT_LOCAL_INFILE is a pointer to unsigned int, so this code is correct. The actual problem is inside PDO, namely

php-src/ext/pdo_mysql/mysql_driver.c

Line 631 in 5951ff7

zend_long local_infile = pdo_attr_lval(driver_options, PDO_MYSQL_ATTR_LOCAL_INFILE, 0);

and

php-src/ext/pdo_mysql/mysql_driver.c

Line 782 in 5951ff7

zend_long local_infile = 0;

These should be changed to use unsigned instead.

[guirish]

@nikic Thanks for sharing the mysql_options function definition, will update local_infile variable to unsigned int type instead of zend_long in mentioned files namely php-src/ext/pdo_mysql/mysql_driver.c and php-src/ext/pdo_mysql/mysql_driver.c.

Also do we need to update global variable allow_local_infile declaration to type unsigned int from ZEND_BEGIN_MODULE_GLOBALS(mysqli) in file php-src/ext/mysqli/php_mysqli_structs.h as mysql_options is too called from file php-src/ext/mysqli/mysqli_nonapi.c specific to MYSQL_OPT_LOCAL_INFILE case?

[nikic]

Also do we need to update global variable allow_local_infile declaration to type unsigned int from ZEND_BEGIN_MODULE_GLOBALS(mysqli) in file php-src/ext/mysqli/php_mysqli_structs.h as mysql_options is too called from file php-src/ext/mysqli/mysqli_nonapi.c specific to MYSQL_OPT_LOCAL_INFILE case?

You're right, that one needs to be changed as well. However, if we want to apply this fix to non-master branches, changing that structure would result in an ABI break. To avoid that, you can just do something like unsigned int allow_local_infile = MyG(allow_local_infile); mysql_options(..., &allow_local_infile) to adjust the type only locally.

[guirish]

@nikic Sure, I will update the method as suggested by you. I will test and push these new changes.

[nikic]

Here again, libmysql specifies STMT_ATTR_CURSOR_TYPE (and also STMT_ATTR_PREFETCH_ROWS below) as unsigned long * (see https://dev.mysql.com/doc/refman/8.0/en/mysql-stmt-attr-set.html). We should use that type here, in the attr_get method directly below, and in

php-src/ext/mysqli/mysqli_api.c

Line 2374 in fd5dc55

PHP_FUNCTION(mysqli_stmt_attr_get)

.

[guirish]

@nikic Kindly correct me if my understanding is wrong.

1. As per the definition you have referred you want me to change the typecasting of value to *(unsigned long * ) value for both the cases i.e STMT_ATTR_CURSOR_TYPE & STMT_ATTR_PREFETCH_ROWS in both the function i.e attr_set and attr_get.
2. I need to change the data type of value variable from zend_ulong to unsigned long in PHP_FUNCTION(mysqli_stmt_attr_get) function to and its function return type accordingly.

Updating diff changes below. Do let me know if any change is required. I have build and tested following changes.

index 80c8ca76fa..0bdfc4f48e 100644
--- a/ext/mysqli/mysqli_api.c
+++ b/ext/mysqli/mysqli_api.c
@@ -2375,7 +2375,7 @@ PHP_FUNCTION(mysqli_stmt_attr_get)
 {
        MY_STMT *stmt;
        zval    *mysql_stmt;
-       zend_ulong      value = 0;
+       unsigned long   value = 0;
        zend_long       attr;
        int             rc;

@@ -2392,7 +2392,7 @@ PHP_FUNCTION(mysqli_stmt_attr_get)
        if (attr == STMT_ATTR_UPDATE_MAX_LENGTH)
                value = *((my_bool *)&value);
 #endif
-       RETURN_LONG((zend_ulong)value);
+       RETURN_LONG((unsigned long)value);
 }
 /* }}} */

diff --git a/ext/mysqlnd/mysqlnd_ps.c b/ext/mysqlnd/mysqlnd_ps.c
index 9d56f6e032..01ee53e0a8 100644
--- a/ext/mysqlnd/mysqlnd_ps.c
+++ b/ext/mysqlnd/mysqlnd_ps.c
@@ -1796,8 +1796,8 @@ MYSQLND_METHOD(mysqlnd_stmt, attr_set)(MYSQLND_STMT * const s,
                        break;
                }
                case STMT_ATTR_CURSOR_TYPE: {
-                       zend_ulong ival = *(zend_ulong *) value;
-                       if (ival > (zend_ulong) CURSOR_TYPE_READ_ONLY) {
+                       unsigned long ival = *(unsigned long *) value;
+                       if (ival > (unsigned long) CURSOR_TYPE_READ_ONLY) {
                                SET_CLIENT_ERROR(stmt->error_info, CR_NOT_IMPLEMENTED, UNKNOWN_SQLSTATE, "Not implemented");
                                DBG_INF("FAIL");
                                DBG_RETURN(FAIL);
@@ -1806,7 +1806,7 @@ MYSQLND_METHOD(mysqlnd_stmt, attr_set)(MYSQLND_STMT * const s,
                        break;
                }
                case STMT_ATTR_PREFETCH_ROWS: {
-                       unsigned int ival = *(unsigned int *) value;
+                       unsigned long ival = *(unsigned long *) value;
                        if (ival == 0) {
                                ival = MYSQLND_DEFAULT_PREFETCH_ROWS;
                        } else if (ival > 1) {
@@ -1845,10 +1845,10 @@ MYSQLND_METHOD(mysqlnd_stmt, attr_get)(const MYSQLND_STMT * const s,
                        *(zend_bool *) value= stmt->update_max_length;
                        break;
                case STMT_ATTR_CURSOR_TYPE:
-                       *(zend_ulong *) value= stmt->flags;
+                       *(unsigned long *) value= stmt->flags;
                        break;
                case STMT_ATTR_PREFETCH_ROWS:
-                       *(zend_ulong *) value= stmt->prefetch_rows;
+                       *(unsigned long *) value= stmt->prefetch_rows;
                        break;
                default:
                        DBG_RETURN(FAIL);

[nikic]

Those changes look right to me. However, there is one more place that needs to be adjusted:

php-src/ext/mysqli/mysqli_api.c

Lines 2349 to 2360 in fd5dc55

	switch (attr) {
	#if MYSQL_VERSION_ID >= 50107
	case STMT_ATTR_UPDATE_MAX_LENGTH:
	mode_b = (my_bool) mode_in;
	mode_p = &mode_b;
	break;
	#endif
	default:
	mode = mode_in;
	mode_p = &mode;
	break;
	}

The mode variable used there should also be unsigned long.

[guirish]

@nikic but mode_in variable is of type zend_long used during assignment?

[nikic]

Yes, but that's fine (can add an explicit cast if you like). The actual values involved here are small, it's just important that the type of the variable whose address we take is correct.

[guirish]

@nikic kindly ignore my above comment, I had not covered the implementation for same. I will update earlier said change and raise new commit after testing.Thanks.