Skip to content

Commit ec7000b

Browse files
nikicnikic
committed
Deprecate boolean return from comparison
But call with reversed operands to make sure it continues working.
1 parent f19d502 commit ec7000b

File tree

5 files changed

+166
-19
lines changed

5 files changed

+166
-19
lines changed

‎ext/spl/tests/bug67539.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ $it = new ArrayIterator(array_fill(0,2,'X'), 1 );
77

88
function badsort($a, $b) {
99
$GLOBALS['it']->unserialize($GLOBALS['it']->serialize());
10-
return TRUE;
10+
return 0;
1111
}
1212

1313
$it->uksort('badsort');

‎ext/standard/array.c

Lines changed: 76 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -923,6 +923,7 @@ static inline int php_array_user_compare_unstable(Bucket *f, Bucket *s) /* {{{ *
923923
{
924924
zval args[2];
925925
zval retval;
926+
zend_bool call_failed;
926927

927928
ZVAL_COPY(&args[0], &f->val);
928929
ZVAL_COPY(&args[1], &s->val);
@@ -931,17 +932,41 @@ static inline int php_array_user_compare_unstable(Bucket *f, Bucket *s) /* {{{ *
931932
BG(user_compare_fci).params = args;
932933
BG(user_compare_fci).retval = &retval;
933934
BG(user_compare_fci).no_separation = 0;
934-
if (zend_call_function(&BG(user_compare_fci), &BG(user_compare_fci_cache)) == SUCCESS && Z_TYPE(retval) != IS_UNDEF) {
935-
zend_long ret = zval_get_long(&retval);
936-
zval_ptr_dtor(&retval);
937-
zval_ptr_dtor(&args[1]);
938-
zval_ptr_dtor(&args[0]);
939-
return ZEND_NORMALIZE_BOOL(ret);
940-
} else {
941-
zval_ptr_dtor(&args[1]);
942-
zval_ptr_dtor(&args[0]);
935+
call_failed = zend_call_function(&BG(user_compare_fci), &BG(user_compare_fci_cache)) == FAILURE || Z_TYPE(retval) == IS_UNDEF;
936+
zval_ptr_dtor(&args[1]);
937+
zval_ptr_dtor(&args[0]);
938+
if (UNEXPECTED(call_failed)) {
943939
return 0;
944940
}
941+
942+
if (UNEXPECTED(Z_TYPE(retval) == IS_FALSE || Z_TYPE(retval) == IS_TRUE)) {
943+
if (!ARRAYG(compare_deprecation_thrown)) {
944+
php_error_docref(NULL, E_DEPRECATED,
945+
"Returning bool from comparison function is deprecated, "
946+
"return one of -1, 0 or 1 instead");
947+
ARRAYG(compare_deprecation_thrown) = 1;
948+
}
949+
950+
if (Z_TYPE(retval) == IS_FALSE) {
951+
/* Retry with swapped operands. */
952+
ZVAL_COPY(&args[0], &s->val);
953+
ZVAL_COPY(&args[1], &f->val);
954+
call_failed = zend_call_function(&BG(user_compare_fci), &BG(user_compare_fci_cache)) == FAILURE || Z_TYPE(retval) == IS_UNDEF;
955+
zval_ptr_dtor(&args[1]);
956+
zval_ptr_dtor(&args[0]);
957+
if (call_failed) {
958+
return 0;
959+
}
960+
961+
zend_long ret = zval_get_long(&retval);
962+
zval_ptr_dtor(&retval);
963+
return -ZEND_NORMALIZE_BOOL(ret);
964+
}
965+
}
966+
967+
zend_long ret = zval_get_long(&retval);
968+
zval_ptr_dtor(&retval);
969+
return ZEND_NORMALIZE_BOOL(ret);
945970
}
946971
/* }}} */
947972

@@ -974,6 +999,7 @@ static int php_array_user_compare(Bucket *a, Bucket *b) /* {{{ */
974999
#define PHP_ARRAY_CMP_FUNC_BACKUP() \
9751000
old_user_compare_fci = BG(user_compare_fci); \
9761001
old_user_compare_fci_cache = BG(user_compare_fci_cache); \
1002+
ARRAYG(compare_deprecation_thrown) = 0; \
9771003
BG(user_compare_fci_cache) = empty_fcall_info_cache; \
9781004

9791005
#define PHP_ARRAY_CMP_FUNC_RESTORE() \
@@ -1033,7 +1059,7 @@ static inline int php_array_user_key_compare_unstable(Bucket *f, Bucket *s) /* {
10331059
{
10341060
zval args[2];
10351061
zval retval;
1036-
zend_long result;
1062+
zend_bool call_failed;
10371063

10381064
if (f->key == NULL) {
10391065
ZVAL_LONG(&args[0], f->h);
@@ -1050,16 +1076,49 @@ static inline int php_array_user_key_compare_unstable(Bucket *f, Bucket *s) /* {
10501076
BG(user_compare_fci).params = args;
10511077
BG(user_compare_fci).retval = &retval;
10521078
BG(user_compare_fci).no_separation = 0;
1053-
if (zend_call_function(&BG(user_compare_fci), &BG(user_compare_fci_cache)) == SUCCESS && Z_TYPE(retval) != IS_UNDEF) {
1054-
result = zval_get_long(&retval);
1055-
zval_ptr_dtor(&retval);
1056-
} else {
1057-
result = 0;
1079+
call_failed = zend_call_function(&BG(user_compare_fci), &BG(user_compare_fci_cache)) == FAILURE || Z_TYPE(retval) == IS_UNDEF;
1080+
zval_ptr_dtor(&args[1]);
1081+
zval_ptr_dtor(&args[0]);
1082+
if (UNEXPECTED(call_failed)) {
1083+
return 0;
10581084
}
10591085

1060-
zval_ptr_dtor(&args[0]);
1061-
zval_ptr_dtor(&args[1]);
1086+
if (UNEXPECTED(Z_TYPE(retval) == IS_FALSE || Z_TYPE(retval) == IS_TRUE)) {
1087+
if (!ARRAYG(compare_deprecation_thrown)) {
1088+
php_error_docref(NULL, E_DEPRECATED,
1089+
"Returning bool from comparison function is deprecated, "
1090+
"return one of -1, 0 or 1 instead");
1091+
ARRAYG(compare_deprecation_thrown) = 1;
1092+
}
1093+
1094+
if (Z_TYPE(retval) == IS_FALSE) {
1095+
/* Retry with swapped operands. */
1096+
if (s->key == NULL) {
1097+
ZVAL_LONG(&args[0], s->h);
1098+
} else {
1099+
ZVAL_STR_COPY(&args[0], s->key);
1100+
}
1101+
if (f->key == NULL) {
1102+
ZVAL_LONG(&args[1], f->h);
1103+
} else {
1104+
ZVAL_STR_COPY(&args[1], f->key);
1105+
}
1106+
1107+
call_failed = zend_call_function(&BG(user_compare_fci), &BG(user_compare_fci_cache)) == FAILURE || Z_TYPE(retval) == IS_UNDEF;
1108+
zval_ptr_dtor(&args[1]);
1109+
zval_ptr_dtor(&args[0]);
1110+
if (call_failed) {
1111+
return 0;
1112+
}
1113+
1114+
zend_long ret = zval_get_long(&retval);
1115+
zval_ptr_dtor(&retval);
1116+
return -ZEND_NORMALIZE_BOOL(ret);
1117+
}
1118+
}
10621119

1120+
zend_long result = zval_get_long(&retval);
1121+
zval_ptr_dtor(&retval);
10631122
return ZEND_NORMALIZE_BOOL(result);
10641123
}
10651124
/* }}} */

‎ext/standard/php_array.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ PHPAPI zend_long php_count_recursive(HashTable *ht);
124124

125125
ZEND_BEGIN_MODULE_GLOBALS(array)
126126
bucket_compare_func_t *multisort_func;
127+
zend_bool compare_deprecation_thrown;
127128
ZEND_END_MODULE_GLOBALS(array)
128129

129130
#define ARRAYG(v) ZEND_MODULE_GLOBALS_ACCESSOR(array, v)

‎ext/standard/tests/array/bug71334.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ class myClass
2121
throw new Exception('Missing Y: "' . $y . '"');
2222
}
2323

24-
return $x < $y;
24+
return $x <=> $y;
2525
}
2626

2727
public function __construct()

‎ext/standard/tests/array/usort_variation11.phpt

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
--TEST--
2+
Test usort() function : usage variations - malformed comparison function returning boolean
3+
--FILE--
4+
<?php
5+
6+
function ucmp($a, $b) {
7+
return $a > $b;
8+
}
9+
10+
$range = array(2, 4, 8, 16, 32, 64, 128);
11+
12+
foreach ($range as $r) {
13+
$backup = $array = range(0, $r);
14+
shuffle($array);
15+
usort($array, "ucmp");
16+
if ($array != $backup) {
17+
var_dump($array);
18+
var_dump($backup);
19+
die("Array not sorted (usort)");
20+
}
21+
22+
shuffle($array);
23+
$array = array_flip($array);
24+
uksort($array, "ucmp");
25+
$array = array_keys($array);
26+
if ($array != $backup) {
27+
var_dump($array);
28+
var_dump($backup);
29+
die("Array not sorted (uksort)");
30+
}
31+
32+
shuffle($array);
33+
$array = array_combine($array, $array);
34+
uasort($array, "ucmp");
35+
$array = array_keys($array);
36+
if ($array != $backup) {
37+
var_dump($array);
38+
var_dump($backup);
39+
die("Array not sorted (uasort)");
40+
}
41+
}
42+
echo "okey";
43+
44+
?>
45+
--EXPECTF--
46+
Deprecated: usort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d
47+
48+
Deprecated: uksort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d
49+
50+
Deprecated: uasort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d
51+
52+
Deprecated: usort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d
53+
54+
Deprecated: uksort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d
55+
56+
Deprecated: uasort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d
57+
58+
Deprecated: usort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d
59+
60+
Deprecated: uksort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d
61+
62+
Deprecated: uasort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d
63+
64+
Deprecated: usort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d
65+
66+
Deprecated: uksort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d
67+
68+
Deprecated: uasort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d
69+
70+
Deprecated: usort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d
71+
72+
Deprecated: uksort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d
73+
74+
Deprecated: uasort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d
75+
76+
Deprecated: usort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d
77+
78+
Deprecated: uksort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d
79+
80+
Deprecated: uasort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d
81+
82+
Deprecated: usort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d
83+
84+
Deprecated: uksort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d
85+
86+
Deprecated: uasort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d
87+
okey

0 commit comments

Comments
 (0)