Skip to content

Commit

Permalink
Revert "Fix GH-9296: ksort behaves incorrectly on arrays with mixed…
Browse files Browse the repository at this point in the history
… keys"

This reverts commit cd1aed8, as
discussed on internals (<https://externals.io/message/118483>).
  • Loading branch information
ramsey committed Aug 30, 2022
1 parent 306da80 commit 5b6a5b0
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 58 deletions.
4 changes: 0 additions & 4 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,6 @@ PHP NEWS
- SQLite3:
. Fixed bug GH-9032 (SQLite3 authorizer crashes on NULL values). (cmb)

- Standard:
. Fixed bug GH-9296 (`ksort` behaves incorrectly on arrays with mixed keys).
(Denis Vaksman)

- Streams:
. Fixed bug GH-8472 (The resource returned by stream_socket_accept may have
incorrect metadata). (Jakub Zelenka)
Expand Down
53 changes: 34 additions & 19 deletions ext/standard/array.c
Original file line number Diff line number Diff line change
Expand Up @@ -166,25 +166,40 @@ static zend_never_inline ZEND_COLD int stable_sort_fallback(Bucket *a, Bucket *b

static zend_always_inline int php_array_key_compare_unstable_i(Bucket *f, Bucket *s) /* {{{ */
{
zval first;
zval second;

if (f->key == NULL && s->key == NULL) {
return (zend_long)f->h > (zend_long)s->h ? 1 : -1;
} else if (f->key && s->key) {
return zendi_smart_strcmp(f->key, s->key);
}
if (f->key) {
ZVAL_STR(&first, f->key);
} else {
ZVAL_LONG(&first, f->h);
}
if (s->key) {
ZVAL_STR(&second, s->key);
} else {
ZVAL_LONG(&second, s->h);
}
return zend_compare(&first, &second);
zend_uchar t;
zend_long l1, l2;
double d;

if (f->key == NULL) {
if (s->key == NULL) {
return (zend_long)f->h > (zend_long)s->h ? 1 : -1;
} else {
l1 = (zend_long)f->h;
t = is_numeric_string(s->key->val, s->key->len, &l2, &d, 1);
if (t == IS_LONG) {
/* pass */
} else if (t == IS_DOUBLE) {
return ZEND_NORMALIZE_BOOL((double)l1 - d);
} else {
l2 = 0;
}
}
} else {
if (s->key) {
return zendi_smart_strcmp(f->key, s->key);
} else {
l2 = (zend_long)s->h;
t = is_numeric_string(f->key->val, f->key->len, &l1, &d, 1);
if (t == IS_LONG) {
/* pass */
} else if (t == IS_DOUBLE) {
return ZEND_NORMALIZE_BOOL(d - (double)l2);
} else {
l1 = 0;
}
}
}
return ZEND_NORMALIZE_BOOL(l1 - l2);
}
/* }}} */

Expand Down
20 changes: 10 additions & 10 deletions ext/standard/tests/array/002.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -276,8 +276,6 @@ array(8) {
-- Testing krsort() --
No second argument:
array(8) {
["test"]=>
int(27)
[16777216]=>
float(-0.3333333333333333)
[1001]=>
Expand All @@ -290,6 +288,8 @@ array(8) {
string(4) "Test"
[0]=>
string(3) "PHP"
["test"]=>
int(27)
[-1000]=>
array(2) {
[0]=>
Expand All @@ -300,8 +300,6 @@ array(8) {
}
Using SORT_REGULAR:
array(8) {
["test"]=>
int(27)
[16777216]=>
float(-0.3333333333333333)
[1001]=>
Expand All @@ -314,6 +312,8 @@ array(8) {
string(4) "Test"
[0]=>
string(3) "PHP"
["test"]=>
int(27)
[-1000]=>
array(2) {
[0]=>
Expand All @@ -334,10 +334,10 @@ array(8) {
string(27) "PHP: Hypertext Preprocessor"
[5]=>
string(4) "Test"
["test"]=>
int(27)
[0]=>
string(3) "PHP"
["test"]=>
int(27)
[-1000]=>
array(2) {
[0]=>
Expand Down Expand Up @@ -383,6 +383,8 @@ array(8) {
}
[0]=>
string(3) "PHP"
["test"]=>
int(27)
[5]=>
string(4) "Test"
[17]=>
Expand All @@ -393,8 +395,6 @@ array(8) {
string(6) "monkey"
[16777216]=>
float(-0.3333333333333333)
["test"]=>
int(27)
}
Using SORT_REGULAR:
array(8) {
Expand All @@ -407,6 +407,8 @@ array(8) {
}
[0]=>
string(3) "PHP"
["test"]=>
int(27)
[5]=>
string(4) "Test"
[17]=>
Expand All @@ -417,8 +419,6 @@ array(8) {
string(6) "monkey"
[16777216]=>
float(-0.3333333333333333)
["test"]=>
int(27)
}
Using SORT_NUMERIC:
array(8) {
Expand Down
17 changes: 0 additions & 17 deletions ext/standard/tests/array/gh9296.phpt

This file was deleted.

8 changes: 4 additions & 4 deletions ext/standard/tests/array/krsort_variation9.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -82,22 +82,22 @@ array(5) {
- With default sort flag -
bool(true)
array(3) {
["c"]=>
string(5) "apple"
["a"]=>
string(6) "orange"
[0]=>
string(6) "banana"
["c"]=>
string(5) "apple"
}
- Sort flag = SORT_REGULAR -
bool(true)
array(3) {
["c"]=>
string(5) "apple"
["a"]=>
string(6) "orange"
[0]=>
string(6) "banana"
["c"]=>
string(5) "apple"
}

-- Iteration 3 --
Expand Down
8 changes: 4 additions & 4 deletions ext/standard/tests/array/ksort_variation9.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -81,20 +81,20 @@ array(5) {
- With default sort flag -
bool(true)
array(3) {
[0]=>
string(6) "banana"
["a"]=>
string(6) "orange"
[0]=>
string(6) "banana"
["c"]=>
string(5) "apple"
}
- Sort flag = SORT_REGULAR -
bool(true)
array(3) {
[0]=>
string(6) "banana"
["a"]=>
string(6) "orange"
[0]=>
string(6) "banana"
["c"]=>
string(5) "apple"
}
Expand Down

0 comments on commit 5b6a5b0

Please sign in to comment.