Skip to content

Commit 0c4501d

Browse files
GirgiasGirgias
committed
Address reviews
1 parent e7307ac commit 0c4501d

13 files changed

+75
-103
lines changed

‎ext/mysqli/mysqli.c

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1024,11 +1024,6 @@ PHP_METHOD(mysqli_result, __construct)
10241024
RETURN_THROWS();
10251025
}
10261026

1027-
if (resmode != MYSQLI_STORE_RESULT && resmode != MYSQLI_USE_RESULT) {
1028-
zend_argument_value_error(2, "must be either MYSQLI_STORE_RESULT or MYSQLI_USE_RESULT");
1029-
RETURN_THROWS();
1030-
}
1031-
10321027
MYSQLI_FETCH_RESOURCE_CONN(mysql, mysql_link, MYSQLI_STATUS_VALID);
10331028

10341029
switch (resmode) {
@@ -1038,7 +1033,9 @@ PHP_METHOD(mysqli_result, __construct)
10381033
case MYSQLI_USE_RESULT:
10391034
result = mysql_use_result(mysql->mysql);
10401035
break;
1041-
EMPTY_SWITCH_DEFAULT_CASE();
1036+
default:
1037+
zend_argument_value_error(2, "must be either MYSQLI_STORE_RESULT or MYSQLI_USE_RESULT");
1038+
RETURN_THROWS();
10421039
}
10431040

10441041
if (!result) {

‎ext/mysqli/mysqli_api.c

Lines changed: 10 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -739,9 +739,8 @@ PHP_FUNCTION(mysqli_data_seek)
739739
MYSQLI_FETCH_RESOURCE(result, MYSQL_RES *, mysql_result, "mysqli_result", MYSQLI_STATUS_VALID);
740740

741741
if (mysqli_result_is_unbuffered(result)) {
742-
// TODO Promote to Exception?
743-
php_error_docref(NULL, E_WARNING, "Function cannot be used with MYSQL_USE_RESULT");
744-
RETURN_FALSE;
742+
zend_throw_error(NULL, "Function cannot be used with MYSQL_USE_RESULT");
743+
RETURN_THROWS();
745744
}
746745

747746
if ((uint64_t)offset >= mysql_num_rows(result)) {
@@ -1197,9 +1196,8 @@ PHP_FUNCTION(mysqli_fetch_field_direct)
11971196
MYSQLI_FETCH_RESOURCE(result, MYSQL_RES *, mysql_result, "mysqli_result", MYSQLI_STATUS_VALID);
11981197

11991198
if (offset >= (zend_long) mysql_num_fields(result)) {
1200-
// TODO ValueError?
1201-
php_error_docref(NULL, E_WARNING, "Field offset is invalid for resultset");
1202-
RETURN_FALSE;
1199+
zend_argument_value_error(ERROR_ARG_POS(2), "must be less than the number of fields for this result set");
1200+
RETURN_THROWS();
12031201
}
12041202

12051203
if (!(field = mysql_fetch_field_direct(result,offset))) {
@@ -1624,9 +1622,8 @@ PHP_FUNCTION(mysqli_num_rows)
16241622
MYSQLI_FETCH_RESOURCE(result, MYSQL_RES *, mysql_result, "mysqli_result", MYSQLI_STATUS_VALID);
16251623

16261624
if (mysqli_result_is_unbuffered_and_not_everything_is_fetched(result)) {
1627-
// TODO Exception?
1628-
php_error_docref(NULL, E_WARNING, "Function cannot be used with MYSQL_USE_RESULT");
1629-
RETURN_LONG(0);
1625+
zend_throw_error(NULL, "Function cannot be used with MYSQL_USE_RESULT");
1626+
RETURN_THROWS();
16301627
}
16311628

16321629
MYSQLI_RETURN_LONG_INT(mysql_num_rows(result));
@@ -2323,25 +2320,18 @@ PHP_FUNCTION(mysqli_stmt_attr_get)
23232320
if (zend_parse_method_parameters(ZEND_NUM_ARGS(), getThis(), "Ol", &mysql_stmt, mysqli_stmt_class_entry, &attr) == FAILURE) {
23242321
RETURN_THROWS();
23252322
}
2323+
23262324
MYSQLI_FETCH_RESOURCE_STMT(stmt, mysql_stmt, MYSQLI_STATUS_VALID);
23272325

2328-
if (attr != STMT_ATTR_CURSOR_TYPE && attr != STMT_ATTR_PREFETCH_ROWS
2329-
#if MYSQL_VERSION_ID >= 50107
2330-
&& attr != STMT_ATTR_UPDATE_MAX_LENGTH
2331-
#endif
2332-
) {
2326+
if ((rc = mysql_stmt_attr_get(stmt->stmt, attr, &value))) {
2327+
/* Success corresponds to 0 return value and a non-zero value
2328+
* should only happen if the attr/option is unknown */
23332329
zend_argument_value_error(ERROR_ARG_POS(2), "must be one of "
23342330
#if MYSQL_VERSION_ID >= 50107
23352331
"MYSQLI_STMT_ATTR_UPDATE_MAX_LENGTH, "
23362332
#endif
23372333
"MYSQLI_STMT_ATTR_PREFETCH_ROWS, or STMT_ATTR_CURSOR_TYPE");
23382334
RETURN_THROWS();
2339-
}
2340-
2341-
if ((rc = mysql_stmt_attr_get(stmt->stmt, attr, &value))) {
2342-
/* Success corresponds to 0 return value and a non-zero value
2343-
* should only happen if the attr/option is unknown */
2344-
ZEND_UNREACHABLE();
23452335
}
23462336

23472337

‎ext/mysqli/mysqli_nonapi.c

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -636,18 +636,13 @@ PHP_FUNCTION(mysqli_query)
636636
RETURN_THROWS();
637637
}
638638
if ((resultmode & ~MYSQLI_ASYNC) != MYSQLI_USE_RESULT &&
639-
MYSQLI_STORE_RESULT !=
640-
#ifdef MYSQLI_USE_MYSQLND
641-
(resultmode & ~(MYSQLI_ASYNC | MYSQLI_STORE_RESULT_COPY_DATA))
642-
#else
643-
(resultmode & ~MYSQLI_ASYNC)
644-
#endif
639+
MYSQLI_STORE_RESULT != (resultmode & ~(MYSQLI_ASYNC | MYSQLI_STORE_RESULT_COPY_DATA))
645640
) {
646-
zend_argument_value_error(ERROR_ARG_POS(3), "must be one of "
641+
zend_argument_value_error(ERROR_ARG_POS(3), "must be either MYSQLI_USE_RESULT, or MYSQLI_STORE_RESULT"
647642
#ifdef MYSQLI_USE_MYSQLND
648-
"MYSQLI_ASYNC,"
643+
" with MYSQLI_ASYNC as an optional bitmask flags"
649644
#endif
650-
"MYSQLI_USE_RESULT, or MYSQLI_STORE_RESULT");
645+
);
651646
RETURN_THROWS();
652647
}
653648

@@ -1172,10 +1167,7 @@ PHP_FUNCTION(mysqli_begin_transaction)
11721167
}
11731168
MYSQLI_FETCH_RESOURCE_CONN(mysql, mysql_link, MYSQLI_STATUS_VALID);
11741169
if (flags < 0) {
1175-
zend_argument_value_error(ERROR_ARG_POS(2), "must be one of MYSQLI_TRANS_START_WITH_CONSISTENT_SNAPSHOT"
1176-
/* TODO Only MySQL 5.6 and later */
1177-
", MYSQLI_TRANS_START_READ_ONLY, or MYSQLI_TRANS_START_READ_WRITE"
1178-
);
1170+
zend_argument_value_error(ERROR_ARG_POS(2), "must be one of the MYSQLI_TRANS_* constant");
11791171
RETURN_THROWS();
11801172
}
11811173
if (!name_len) {

‎ext/mysqli/tests/bug55582.phpt

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,27 +15,31 @@ require_once("connect.inc");
1515

1616
var_dump($link->real_query("SELECT 1"));
1717
$res = $link->use_result();
18-
var_dump(mysqli_num_rows($res));
18+
try {
19+
var_dump(mysqli_num_rows($res));
20+
} catch (\Error $e) {
21+
echo $e->getMessage() . \PHP_EOL;
22+
}
1923
var_dump($res->fetch_assoc());
20-
var_dump(mysqli_num_rows($res));
24+
try {
25+
var_dump(mysqli_num_rows($res));
26+
} catch (\Error $e) {
27+
echo $e->getMessage() . \PHP_EOL;
28+
}
2129
var_dump($res->fetch_assoc());
2230
var_dump(mysqli_num_rows($res));
2331

2432
$link->close();
2533
echo "done\n";
2634
?>
27-
--EXPECTF--
35+
--EXPECT--
2836
bool(true)
29-
30-
Warning: mysqli_num_rows(): Function cannot be used with MYSQL_USE_RESULT in %s on line %d
31-
int(0)
37+
Function cannot be used with MYSQL_USE_RESULT
3238
array(1) {
3339
[1]=>
3440
string(1) "1"
3541
}
36-
37-
Warning: mysqli_num_rows(): Function cannot be used with MYSQL_USE_RESULT in %s on line %d
38-
int(0)
42+
Function cannot be used with MYSQL_USE_RESULT
3943
NULL
4044
int(1)
4145
done

‎ext/mysqli/tests/bug74595.phpt

Lines changed: 0 additions & 25 deletions
This file was deleted.

‎ext/mysqli/tests/mysqli_begin_transaction.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,5 +101,5 @@ if (!have_innodb($link))
101101
?>
102102
--EXPECT--
103103
NULL
104-
mysqli_begin_transaction(): Argument #2 ($flags) must be one of MYSQLI_TRANS_START_WITH_CONSISTENT_SNAPSHOT, MYSQLI_TRANS_START_READ_ONLY, or MYSQLI_TRANS_START_READ_WRITE
104+
mysqli_begin_transaction(): Argument #2 ($flags) must be one of the MYSQLI_TRANS_* constant
105105
done!

‎ext/mysqli/tests/mysqli_data_seek.phpt

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,11 @@ require_once('skipifconnectfailure.inc');
4242
if (!$res = mysqli_query($link, 'SELECT * FROM test ORDER BY id', MYSQLI_USE_RESULT))
4343
printf("[011] [%d] %s\n", mysqli_errno($link), mysqli_error($link));
4444

45-
if (false !== ($tmp = mysqli_data_seek($res, 3)))
46-
printf("[012] Expecting boolean/false, got %s/%s\n", gettype($tmp), $tmp);
45+
try {
46+
var_dump(mysqli_data_seek($res, 3));
47+
} catch (\Error $e) {
48+
echo $e->getMessage() . \PHP_EOL;
49+
}
4750

4851
mysqli_free_result($res);
4952

@@ -61,9 +64,8 @@ require_once('skipifconnectfailure.inc');
6164
<?php
6265
require_once("clean_table.inc");
6366
?>
64-
--EXPECTF--
67+
--EXPECT--
6568
mysqli_data_seek(): Argument #2 ($offset) must be greater than or equal to 0
66-
67-
Warning: mysqli_data_seek(): Function cannot be used with MYSQL_USE_RESULT in %s on line %d
69+
Function cannot be used with MYSQL_USE_RESULT
6870
mysqli_result object is already closed
6971
done!

‎ext/mysqli/tests/mysqli_data_seek_oo.phpt

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,11 @@ require_once('skipifconnectfailure.inc');
5454
if (!$res = $mysqli->query('SELECT * FROM test ORDER BY id', MYSQLI_USE_RESULT))
5555
printf("[013] [%d] %s\n", mysqli_errno($link), mysqli_error($link));
5656

57-
if (false !== ($tmp = $res->data_seek(3)))
58-
printf("[014] Expecting boolean/false, got %s/%s\n", gettype($tmp), $tmp);
57+
try {
58+
var_dump($res->data_seek(3));
59+
} catch (\Error $e) {
60+
echo $e->getMessage() . \PHP_EOL;
61+
}
5962

6063
$res->free_result();
6164

@@ -72,10 +75,9 @@ require_once('skipifconnectfailure.inc');
7275
<?php
7376
require_once("clean_table.inc");
7477
?>
75-
--EXPECTF--
78+
--EXPECT--
7679
mysqli_result object is already closed
7780
mysqli_result::data_seek(): Argument #1 ($offset) must be greater than or equal to 0
78-
79-
Warning: mysqli_result::data_seek(): Function cannot be used with MYSQL_USE_RESULT in %s on line %d
81+
Function cannot be used with MYSQL_USE_RESULT
8082
mysqli_result object is already closed
8183
done!

‎ext/mysqli/tests/mysqli_fetch_field_direct.phpt

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,12 @@ require_once('skipifconnectfailure.inc');
2222
echo $e->getMessage() . \PHP_EOL;
2323
}
2424
var_dump(mysqli_fetch_field_direct($res, 0));
25-
var_dump(mysqli_fetch_field_direct($res, 2));
25+
26+
try {
27+
var_dump(mysqli_fetch_field_direct($res, 2));
28+
} catch (\ValueError $e) {
29+
echo $e->getMessage() . \PHP_EOL;
30+
}
2631

2732
mysqli_free_result($res);
2833

@@ -69,8 +74,6 @@ object(stdClass)#%d (13) {
6974
["decimals"]=>
7075
int(%d)
7176
}
72-
73-
Warning: mysqli_fetch_field_direct(): Field offset is invalid for resultset in %s on line %d
74-
bool(false)
77+
mysqli_fetch_field_direct(): Argument #2 ($offset) must be less than the number of fields for this result set
7578
mysqli_result object is already closed
7679
done!

‎ext/mysqli/tests/mysqli_fetch_field_direct_oo.phpt

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,12 @@ require_once('skipifconnectfailure.inc');
3434
}
3535

3636
var_dump($res->fetch_field_direct(0));
37-
var_dump($res->fetch_field_direct(2));
37+
38+
try {
39+
var_dump($res->fetch_field_direct(2));
40+
} catch (\ValueError $e) {
41+
echo $e->getMessage() . \PHP_EOL;
42+
}
3843

3944
$res->free_result();
4045

@@ -82,8 +87,6 @@ object(stdClass)#%d (13) {
8287
["decimals"]=>
8388
int(%d)
8489
}
85-
86-
Warning: mysqli_result::fetch_field_direct(): Field offset is invalid for resultset in %s on line %d
87-
bool(false)
90+
mysqli_result::fetch_field_direct(): Argument #1 ($field_nr) must be less than the number of fields for this result set
8891
mysqli_result object is already closed
8992
done!

‎ext/mysqli/tests/mysqli_num_rows.phpt

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,11 @@ require_once('skipifconnectfailure.inc');
5656
if ($res = mysqli_query($link, 'SELECT id FROM test', MYSQLI_USE_RESULT)) {
5757

5858
$row = mysqli_fetch_row($res);
59-
if (0 !== ($tmp = mysqli_num_rows($res)))
60-
printf("[031] Expecting int/0, got %s/%d\n", gettype($tmp), $tmp);
59+
try {
60+
var_dump(mysqli_num_rows($res));
61+
} catch (\Error $e) {
62+
echo $e->getMessage() . \PHP_EOL;
63+
}
6164

6265
mysqli_free_result($res);
6366
} else {
@@ -71,12 +74,11 @@ require_once('skipifconnectfailure.inc');
7174
<?php
7275
require_once("clean_table.inc");
7376
?>
74-
--EXPECTF--
77+
--EXPECT--
7578
mysqli_result object is already closed
7679
mysqli_result object is already closed
7780
mysqli_result object is already closed
7881
mysqli_result object is already closed
7982
run_tests.php don't fool me with your 'ungreedy' expression '.+?'!
80-
81-
Warning: mysqli_num_rows(): Function cannot be used with MYSQL_USE_RESULT in %s on line %d
83+
Function cannot be used with MYSQL_USE_RESULT
8284
done!

‎ext/mysqli/tests/mysqli_query.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,6 @@ array(1) {
126126
string(1) "a"
127127
}
128128
string(1) "a"
129-
mysqli_query(): Argument #3 ($result_mode) must be one of MYSQLI_ASYNC,MYSQLI_USE_RESULT, or MYSQLI_STORE_RESULT
129+
mysqli_query(): Argument #3 ($result_mode) must be either MYSQLI_USE_RESULT, or MYSQLI_STORE_RESULT with MYSQLI_ASYNC as an optional bitmask flags
130130
mysqli object is already closed
131131
done!

‎ext/mysqli/tests/mysqli_use_result.phpt

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,11 @@ require_once('skipifconnectfailure.inc');
1919
printf("[004] Expecting object, got %s/%s. [%d] %s\n",
2020
gettype($res), $res, mysqli_errno($link), mysqli_error($link));
2121

22-
if (false !== ($tmp = mysqli_data_seek($res, 2)))
23-
printf("[005] Expecting boolean/true, got %s/%s. [%d] %s\n",
24-
gettype($tmp), $tmp, mysqli_errno($link), mysqli_error($link));
22+
try {
23+
var_dump(mysqli_data_seek($res, 2));
24+
} catch (\Error $e) {
25+
echo $e->getMessage() . \PHP_EOL;
26+
}
2527

2628
mysqli_free_result($res);
2729

@@ -53,7 +55,7 @@ require_once('skipifconnectfailure.inc');
5355
<?php
5456
require_once("clean_table.inc");
5557
?>
56-
--EXPECTF--
57-
Warning: mysqli_data_seek(): Function cannot be used with MYSQL_USE_RESULT in %s on line %d
58+
--EXPECT--
59+
Function cannot be used with MYSQL_USE_RESULT
5860
mysqli object is already closed
5961
done!

0 commit comments

Comments
 (0)