Skip to content

Commit 9b6d7f5

Browse files
morrisonlevinikic
morrisonlevi
authored and
nikic
committed
Remove extra pointer in SplFixedArray
1 parent e5d9d00 commit 9b6d7f5

File tree

2 files changed

+62
-91
lines changed

2 files changed

+62
-91
lines changed

‎ext/spl/spl_fixedarray.c

Lines changed: 57 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ typedef struct _spl_fixedarray { /* {{{ */
4949
/* }}} */
5050

5151
typedef struct _spl_fixedarray_object { /* {{{ */
52-
spl_fixedarray *array;
52+
spl_fixedarray array;
5353
zend_function *fptr_offset_get;
5454
zend_function *fptr_offset_set;
5555
zend_function *fptr_offset_has;
@@ -148,13 +148,8 @@ static HashTable* spl_fixedarray_object_get_gc(zval *obj, zval **table, int *n)
148148
spl_fixedarray_object *intern = Z_SPLFIXEDARRAY_P(obj);
149149
HashTable *ht = zend_std_get_properties(obj);
150150

151-
if (intern->array) {
152-
*table = intern->array->elements;
153-
*n = (int)intern->array->size;
154-
} else {
155-
*table = NULL;
156-
*n = 0;
157-
}
151+
*table = intern->array.elements;
152+
*n = (int)intern->array.size;
158153

159154
return ht;
160155
}
@@ -166,21 +161,21 @@ static HashTable* spl_fixedarray_object_get_properties(zval *obj) /* {{{{ */
166161
HashTable *ht = zend_std_get_properties(obj);
167162
zend_long i = 0;
168163

169-
if (intern->array) {
164+
if (intern->array.size > 0) {
170165
zend_long j = zend_hash_num_elements(ht);
171166

172-
for (i = 0; i < intern->array->size; i++) {
173-
if (!Z_ISUNDEF(intern->array->elements[i])) {
174-
zend_hash_index_update(ht, i, &intern->array->elements[i]);
175-
if (Z_REFCOUNTED(intern->array->elements[i])){
176-
Z_ADDREF(intern->array->elements[i]);
167+
for (i = 0; i < intern->array.size; i++) {
168+
if (!Z_ISUNDEF(intern->array.elements[i])) {
169+
zend_hash_index_update(ht, i, &intern->array.elements[i]);
170+
if (Z_REFCOUNTED(intern->array.elements[i])){
171+
Z_ADDREF(intern->array.elements[i]);
177172
}
178173
} else {
179174
zend_hash_index_update(ht, i, &EG(uninitialized_zval));
180175
}
181176
}
182-
if (j > intern->array->size) {
183-
for (i = intern->array->size; i < j; ++i) {
177+
if (j > intern->array.size) {
178+
for (i = intern->array.size; i < j; ++i) {
184179
zend_hash_index_del(ht, i);
185180
}
186181
}
@@ -195,15 +190,14 @@ static void spl_fixedarray_object_free_storage(zend_object *object) /* {{{ */
195190
spl_fixedarray_object *intern = spl_fixed_array_from_obj(object);
196191
zend_long i;
197192

198-
if (intern->array) {
199-
for (i = 0; i < intern->array->size; i++) {
200-
zval_ptr_dtor(&(intern->array->elements[i]));
193+
if (intern->array.size > 0) {
194+
for (i = 0; i < intern->array.size; i++) {
195+
zval_ptr_dtor(&(intern->array.elements[i]));
201196
}
202197

203-
if (intern->array->size > 0 && intern->array->elements) {
204-
efree(intern->array->elements);
198+
if (intern->array.size > 0 && intern->array.elements) {
199+
efree(intern->array.elements);
205200
}
206-
efree(intern->array);
207201
}
208202

209203
zend_object_std_dtor(&intern->std);
@@ -229,14 +223,8 @@ static zend_object *spl_fixedarray_object_new_ex(zend_class_entry *class_type, z
229223
if (orig && clone_orig) {
230224
spl_fixedarray_object *other = Z_SPLFIXEDARRAY_P(orig);
231225
intern->ce_get_iterator = other->ce_get_iterator;
232-
if (!other->array) {
233-
/* leave a empty object, will be dtor later by CLONE handler */
234-
zend_throw_exception(spl_ce_RuntimeException, "The instance wasn't initialized properly", 0);
235-
} else {
236-
intern->array = emalloc(sizeof(spl_fixedarray));
237-
spl_fixedarray_init(intern->array, other->array->size);
238-
spl_fixedarray_copy(intern->array, other->array);
239-
}
226+
spl_fixedarray_init(&intern->array, other->array.size);
227+
spl_fixedarray_copy(&intern->array, &other->array);
240228
}
241229

242230
while (parent) {
@@ -341,13 +329,13 @@ static inline zval *spl_fixedarray_object_read_dimension_helper(spl_fixedarray_o
341329
index = Z_LVAL_P(offset);
342330
}
343331

344-
if (index < 0 || intern->array == NULL || index >= intern->array->size) {
332+
if (index < 0 || index >= intern->array.size) {
345333
zend_throw_exception(spl_ce_RuntimeException, "Index invalid or out of range", 0);
346334
return NULL;
347-
} else if (Z_ISUNDEF(intern->array->elements[index])) {
335+
} else if (Z_ISUNDEF(intern->array.elements[index])) {
348336
return NULL;
349337
} else {
350-
return &intern->array->elements[index];
338+
return &intern->array.elements[index];
351339
}
352340
}
353341
/* }}} */
@@ -394,15 +382,15 @@ static inline void spl_fixedarray_object_write_dimension_helper(spl_fixedarray_o
394382
index = Z_LVAL_P(offset);
395383
}
396384

397-
if (index < 0 || intern->array == NULL || index >= intern->array->size) {
385+
if (index < 0 || index >= intern->array.size) {
398386
zend_throw_exception(spl_ce_RuntimeException, "Index invalid or out of range", 0);
399387
return;
400388
} else {
401-
if (!Z_ISUNDEF(intern->array->elements[index])) {
402-
zval_ptr_dtor(&(intern->array->elements[index]));
389+
if (!Z_ISUNDEF(intern->array.elements[index])) {
390+
zval_ptr_dtor(&(intern->array.elements[index]));
403391
}
404392
ZVAL_DEREF(value);
405-
ZVAL_COPY(&intern->array->elements[index], value);
393+
ZVAL_COPY(&intern->array.elements[index], value);
406394
}
407395
}
408396
/* }}} */
@@ -442,12 +430,12 @@ static inline void spl_fixedarray_object_unset_dimension_helper(spl_fixedarray_o
442430
index = Z_LVAL_P(offset);
443431
}
444432

445-
if (index < 0 || intern->array == NULL || index >= intern->array->size) {
433+
if (index < 0 || index >= intern->array.size) {
446434
zend_throw_exception(spl_ce_RuntimeException, "Index invalid or out of range", 0);
447435
return;
448436
} else {
449-
zval_ptr_dtor(&(intern->array->elements[index]));
450-
ZVAL_UNDEF(&intern->array->elements[index]);
437+
zval_ptr_dtor(&(intern->array.elements[index]));
438+
ZVAL_UNDEF(&intern->array.elements[index]);
451439
}
452440
}
453441
/* }}} */
@@ -481,13 +469,13 @@ static inline int spl_fixedarray_object_has_dimension_helper(spl_fixedarray_obje
481469
index = Z_LVAL_P(offset);
482470
}
483471

484-
if (index < 0 || intern->array == NULL || index >= intern->array->size) {
472+
if (index < 0 || index >= intern->array.size) {
485473
retval = 0;
486474
} else {
487-
if (Z_ISUNDEF(intern->array->elements[index])) {
475+
if (Z_ISUNDEF(intern->array.elements[index])) {
488476
retval = 0;
489477
} else if (check_empty) {
490-
if (zend_is_true(&intern->array->elements[index])) {
478+
if (zend_is_true(&intern->array.elements[index])) {
491479
retval = 1;
492480
} else {
493481
retval = 0;
@@ -535,14 +523,12 @@ static int spl_fixedarray_object_count_elements(zval *object, zend_long *count)
535523
if (!Z_ISUNDEF(rv)) {
536524
*count = zval_get_long(&rv);
537525
zval_ptr_dtor(&rv);
538-
return SUCCESS;
526+
} else {
527+
*count = 0;
539528
}
540-
} else if (intern->array) {
541-
*count = intern->array->size;
542-
return SUCCESS;
529+
} else {
530+
*count = intern->array.size;
543531
}
544-
545-
*count = 0;
546532
return SUCCESS;
547533
}
548534
/* }}} */
@@ -566,13 +552,12 @@ SPL_METHOD(SplFixedArray, __construct)
566552

567553
intern = Z_SPLFIXEDARRAY_P(object);
568554

569-
if (intern->array) {
555+
if (intern->array.size > 0) {
570556
/* called __construct() twice, bail out */
571557
return;
572558
}
573559

574-
intern->array = emalloc(sizeof(spl_fixedarray));
575-
spl_fixedarray_init(intern->array, size);
560+
spl_fixedarray_init(&intern->array, size);
576561
}
577562
/* }}} */
578563

@@ -588,18 +573,17 @@ SPL_METHOD(SplFixedArray, __wakeup)
588573
return;
589574
}
590575

591-
if (!intern->array) {
576+
if (intern->array.size == 0) {
592577
int index = 0;
593578
int size = zend_hash_num_elements(intern_ht);
594579

595-
intern->array = emalloc(sizeof(spl_fixedarray));
596-
spl_fixedarray_init(intern->array, size);
580+
spl_fixedarray_init(&intern->array, size);
597581

598582
ZEND_HASH_FOREACH_VAL(intern_ht, data) {
599583
if (Z_REFCOUNTED_P(data)) {
600584
Z_ADDREF_P(data);
601585
}
602-
ZVAL_COPY_VALUE(&intern->array->elements[index], data);
586+
ZVAL_COPY_VALUE(&intern->array.elements[index], data);
603587
index++;
604588
} ZEND_HASH_FOREACH_END();
605589

@@ -622,10 +606,7 @@ SPL_METHOD(SplFixedArray, count)
622606
}
623607

624608
intern = Z_SPLFIXEDARRAY_P(object);
625-
if (intern->array) {
626-
RETURN_LONG(intern->array->size);
627-
}
628-
RETURN_LONG(0);
609+
RETURN_LONG(intern->array.size);
629610
}
630611
/* }}} */
631612

@@ -642,13 +623,13 @@ SPL_METHOD(SplFixedArray, toArray)
642623
intern = Z_SPLFIXEDARRAY_P(getThis());
643624

644625
array_init(return_value);
645-
if (intern->array) {
626+
if (intern->array.size > 0) {
646627
int i = 0;
647-
for (; i < intern->array->size; i++) {
648-
if (!Z_ISUNDEF(intern->array->elements[i])) {
649-
zend_hash_index_update(Z_ARRVAL_P(return_value), i, &intern->array->elements[i]);
650-
if (Z_REFCOUNTED(intern->array->elements[i])) {
651-
Z_ADDREF(intern->array->elements[i]);
628+
for (; i < intern->array.size; i++) {
629+
if (!Z_ISUNDEF(intern->array.elements[i])) {
630+
zend_hash_index_update(Z_ARRVAL_P(return_value), i, &intern->array.elements[i]);
631+
if (Z_REFCOUNTED(intern->array.elements[i])) {
632+
Z_ADDREF(intern->array.elements[i]);
652633
}
653634
} else {
654635
zend_hash_index_update(Z_ARRVAL_P(return_value), i, &EG(uninitialized_zval));
@@ -663,7 +644,7 @@ SPL_METHOD(SplFixedArray, toArray)
663644
SPL_METHOD(SplFixedArray, fromArray)
664645
{
665646
zval *data;
666-
spl_fixedarray *array;
647+
spl_fixedarray array;
667648
spl_fixedarray_object *intern;
668649
int num;
669650
zend_bool save_indexes = 1;
@@ -672,7 +653,6 @@ SPL_METHOD(SplFixedArray, fromArray)
672653
return;
673654
}
674655

675-
array = ecalloc(1, sizeof(spl_fixedarray));
676656
num = zend_hash_num_elements(Z_ARRVAL_P(data));
677657

678658
if (num > 0 && save_indexes) {
@@ -683,7 +663,6 @@ SPL_METHOD(SplFixedArray, fromArray)
683663

684664
ZEND_HASH_FOREACH_KEY(Z_ARRVAL_P(data), num_index, str_index) {
685665
if (str_index != NULL || (zend_long)num_index < 0) {
686-
efree(array);
687666
zend_throw_exception_ex(spl_ce_InvalidArgumentException, 0, "array must contain only positive integer keys");
688667
return;
689668
}
@@ -695,30 +674,29 @@ SPL_METHOD(SplFixedArray, fromArray)
695674

696675
tmp = max_index + 1;
697676
if (tmp <= 0) {
698-
efree(array);
699677
zend_throw_exception_ex(spl_ce_InvalidArgumentException, 0, "integer overflow detected");
700678
return;
701679
}
702-
spl_fixedarray_init(array, tmp);
680+
spl_fixedarray_init(&array, tmp);
703681

704682
ZEND_HASH_FOREACH_KEY_VAL(Z_ARRVAL_P(data), num_index, str_index, element) {
705683
ZVAL_DEREF(element);
706-
ZVAL_COPY(&array->elements[num_index], element);
684+
ZVAL_COPY(&array.elements[num_index], element);
707685
} ZEND_HASH_FOREACH_END();
708686

709687
} else if (num > 0 && !save_indexes) {
710688
zval *element;
711689
zend_long i = 0;
712690

713-
spl_fixedarray_init(array, num);
691+
spl_fixedarray_init(&array, num);
714692

715693
ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(data), element) {
716694
ZVAL_DEREF(element);
717-
ZVAL_COPY(&array->elements[i], element);
695+
ZVAL_COPY(&array.elements[i], element);
718696
i++;
719697
} ZEND_HASH_FOREACH_END();
720698
} else {
721-
spl_fixedarray_init(array, 0);
699+
spl_fixedarray_init(&array, 0);
722700
}
723701

724702
object_init_ex(return_value, spl_ce_SplFixedArray);
@@ -740,10 +718,7 @@ SPL_METHOD(SplFixedArray, getSize)
740718
}
741719

742720
intern = Z_SPLFIXEDARRAY_P(object);
743-
if (intern->array) {
744-
RETURN_LONG(intern->array->size);
745-
}
746-
RETURN_LONG(0);
721+
RETURN_LONG(intern->array.size);
747722
}
748723
/* }}} */
749724

@@ -765,11 +740,8 @@ SPL_METHOD(SplFixedArray, setSize)
765740
}
766741

767742
intern = Z_SPLFIXEDARRAY_P(object);
768-
if (!intern->array) {
769-
intern->array = ecalloc(1, sizeof(spl_fixedarray));
770-
}
771743

772-
spl_fixedarray_resize(intern->array, size);
744+
spl_fixedarray_resize(&intern->array, size);
773745
RETURN_TRUE;
774746
}
775747
/* }}} */
@@ -873,7 +845,7 @@ static int spl_fixedarray_it_valid(zend_object_iterator *iter) /* {{{ */
873845
return zend_user_it_valid(iter);
874846
}
875847

876-
if (object->current >= 0 && object->array && object->current < object->array->size) {
848+
if (object->current >= 0 && object->current < object->array.size) {
877849
return SUCCESS;
878850
}
879851

@@ -967,7 +939,7 @@ SPL_METHOD(SplFixedArray, valid)
967939
return;
968940
}
969941

970-
RETURN_BOOL(intern->current >= 0 && intern->array && intern->current < intern->array->size);
942+
RETURN_BOOL(intern->current >= 0 && intern->current < intern->array.size);
971943
}
972944
/* }}} */
973945

‎ext/spl/tests/bug62904.phpt

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,9 @@ class foo extends SplFixedArray {
1010

1111
$x = new foo(2);
1212

13-
try {
14-
$z = clone $x;
15-
} catch (Exception $e) {
16-
var_dump($e->getMessage());
17-
}
13+
$z = clone $x;
14+
echo "No crash.";
15+
1816
--EXPECTF--
19-
string(40) "The instance wasn't initialized properly"
17+
No crash.
18+

0 commit comments

Comments
 (0)