Skip to content

Commit 9bc84f1

Browse files
kocsismatekocsismate
committed
Address code review
1 parent 845abb4 commit 9bc84f1

13 files changed

+96
-94
lines changed

‎ext/com_dotnet/com_com.c

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -684,34 +684,35 @@ PHP_FUNCTION(com_create_guid)
684684
/* {{{ Connect events from a COM object to a PHP object */
685685
PHP_FUNCTION(com_event_sink)
686686
{
687-
zval *object, *sinkobject, *sink=NULL;
687+
zval *object, *sinkobject;
688+
zend_string *sink_str = NULL;
689+
HashTable *sink_ht = NULL;
688690
char *dispname = NULL, *typelibname = NULL;
689691
php_com_dotnet_object *obj;
690692
ITypeInfo *typeinfo = NULL;
691693

692-
RETVAL_FALSE;
694+
ZEND_PARSE_PARAMETERS_START(2, 3)
695+
Z_PARAM_OBJECT_OF_CLASS(object, php_com_variant_class_entry)
696+
Z_PARAM_OBJECT(sinkobject)
697+
Z_PARAM_OPTIONAL
698+
Z_PARAM_STR_OR_ARRAY_HT_OR_NULL(sink_str, sink_ht)
699+
ZEND_PARSE_PARAMETERS_END();
693700

694-
if (FAILURE == zend_parse_parameters(ZEND_NUM_ARGS(), "Oo|z!/",
695-
&object, php_com_variant_class_entry, &sinkobject, &sink)) {
696-
RETURN_THROWS();
697-
}
701+
RETVAL_FALSE;
698702

699703
php_com_initialize();
700704
obj = CDNO_FETCH(object);
701705

702-
if (sink && Z_TYPE_P(sink) == IS_ARRAY) {
706+
if (sink_ht) {
703707
/* 0 => typelibname, 1 => dispname */
704708
zval *tmp;
705709

706-
if ((tmp = zend_hash_index_find(Z_ARRVAL_P(sink), 0)) != NULL && Z_TYPE_P(tmp) == IS_STRING)
710+
if ((tmp = zend_hash_index_find(sink_ht, 0)) != NULL && Z_TYPE_P(tmp) == IS_STRING)
707711
typelibname = Z_STRVAL_P(tmp);
708-
if ((tmp = zend_hash_index_find(Z_ARRVAL_P(sink), 1)) != NULL && Z_TYPE_P(tmp) == IS_STRING)
712+
if ((tmp = zend_hash_index_find(sink_ht, 1)) != NULL && Z_TYPE_P(tmp) == IS_STRING)
709713
dispname = Z_STRVAL_P(tmp);
710-
} else if (sink != NULL) {
711-
if (!try_convert_to_string(sink)) {
712-
RETURN_THROWS();
713-
}
714-
dispname = Z_STRVAL_P(sink);
714+
} else if (sink_str) {
715+
dispname = ZSTR_VAL(sink_str);
715716
}
716717

717718
typeinfo = php_com_locate_typeinfo(typelibname, obj, dispname, 1);

‎ext/com_dotnet/com_extension.stub.php

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,7 @@ function com_get_active_object(string $progid, ?int $code_page = null): variant
5858

5959
function com_create_guid(): string|false {}
6060

61-
/** @param array|string|null $sinkinterface */
62-
function com_event_sink(variant $comobject, object $sinkobject, $sinkinterface = UNKNOWN): bool {}
61+
function com_event_sink(variant $comobject, object $sinkobject, array|string|null $sinkinterface = null): bool {}
6362

6463
/** @param com|dotnet|variant|string $comobject */
6564
function com_print_typeinfo($comobject, ?string $dispinterface = null, bool $wantsink = false): bool {}

‎ext/com_dotnet/com_extension_arginfo.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/* This is a generated file, edit the .stub.php file instead.
2-
* Stub hash: 9c7d872d61142127560453283a1ca9f07bfdc207 */
2+
* Stub hash: f78e9db58131f9d67021eaea4c3d4be75cafe2ac */
33

44
ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_variant_set, 0, 2, IS_VOID, 0)
55
ZEND_ARG_OBJ_INFO(0, variant, variant, 0)
@@ -92,7 +92,7 @@ ZEND_END_ARG_INFO()
9292
ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_com_event_sink, 0, 2, _IS_BOOL, 0)
9393
ZEND_ARG_OBJ_INFO(0, comobject, variant, 0)
9494
ZEND_ARG_TYPE_INFO(0, sinkobject, IS_OBJECT, 0)
95-
ZEND_ARG_INFO(0, sinkinterface)
95+
ZEND_ARG_TYPE_MASK(0, sinkinterface, MAY_BE_ARRAY|MAY_BE_STRING|MAY_BE_NULL, "null")
9696
ZEND_END_ARG_INFO()
9797

9898
ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_com_print_typeinfo, 0, 1, _IS_BOOL, 0)

‎ext/intl/calendar/calendar_methods.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1020,7 +1020,7 @@ U_CFUNC PHP_FUNCTION(intlcal_from_date_time)
10201020
calendar_object_create(return_value, cal);
10211021

10221022
error:
1023-
if (date_obj && date_str) {
1023+
if (date_str) {
10241024
OBJ_RELEASE(date_obj);
10251025
}
10261026
}

‎ext/intl/intl_data.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,16 @@ typedef struct _intl_data {
7171
RETURN_FALSE; \
7272
}
7373

74+
/* Check status in object, if error goto a label */
75+
#define INTL_METHOD_CHECK_STATUS_OR_GOTO(obj, msg, label) \
76+
intl_error_set_code( NULL, INTL_DATA_ERROR_CODE((obj)) ); \
77+
if( U_FAILURE( INTL_DATA_ERROR_CODE((obj)) ) ) \
78+
{ \
79+
intl_errors_set_custom_msg( INTL_DATA_ERROR_P((obj)), msg, 0 ); \
80+
RETVAL_FALSE; \
81+
goto label; \
82+
}
83+
7484
/* Check status in object, if error return null */
7585
#define INTL_METHOD_CHECK_STATUS_OR_NULL(obj, msg) \
7686
intl_error_set_code( NULL, INTL_DATA_ERROR_CODE((obj)) ); \

‎ext/intl/transliterator/transliterator_methods.c

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -280,12 +280,13 @@ PHP_FUNCTION( transliterator_transliterate )
280280
TRANSLITERATOR_METHOD_INIT_VARS;
281281

282282
object = getThis();
283+
283284
ZVAL_UNDEF(&tmp_object);
284285

285286
if (object == NULL) {
286287
/* in non-OOP version, accept both a transliterator and a string */
287-
zend_string *arg1_str;
288-
zend_object *arg1_obj;
288+
zend_string *arg1_str = NULL;
289+
zend_object *arg1_obj = NULL;
289290

290291
ZEND_PARSE_PARAMETERS_START(2, 4)
291292
Z_PARAM_STR_OR_OBJ_OF_CLASS(arg1_str, arg1_obj, Transliterator_ce_ptr)
@@ -295,6 +296,8 @@ PHP_FUNCTION( transliterator_transliterate )
295296
Z_PARAM_LONG(limit)
296297
ZEND_PARSE_PARAMETERS_END();
297298

299+
ZVAL_UNDEF(&tmp_object);
300+
298301
if (arg1_str) { /* not a transliterator object as first argument */
299302
int res;
300303
object = &tmp_object;
@@ -310,42 +313,41 @@ PHP_FUNCTION( transliterator_transliterate )
310313
goto cleanup;
311314
}
312315
} else {
313-
ZVAL_OBJ(&tmp_object, arg1_obj);
316+
ZVAL_OBJ_COPY(&tmp_object, arg1_obj);
314317
object = &tmp_object;
315318
}
316-
} else if( zend_parse_parameters( ZEND_NUM_ARGS(), "s|ll",
317-
&str, &str_len, &start, &limit ) == FAILURE )
318-
{
319+
} else if(zend_parse_parameters( ZEND_NUM_ARGS(), "s|ll", &str, &str_len, &start, &limit) == FAILURE) {
319320
RETURN_THROWS();
320321
}
321322

322-
if( limit < -1 )
323+
if( limit < -1 ) /* leaks */
323324
{
324325
intl_error_set( NULL, U_ILLEGAL_ARGUMENT_ERROR,
325326
"transliterator_transliterate: \"end\" argument should be "
326327
"either non-negative or -1", 0 );
327-
RETURN_FALSE;
328+
RETVAL_FALSE;
329+
goto cleanup_object;
328330
}
329331

330-
if( start < 0 || ((limit != -1 ) && (start > limit )) )
332+
if( start < 0 || ((limit != -1 ) && (start > limit )) ) /* leaks */
331333
{
332334
intl_error_set( NULL, U_ILLEGAL_ARGUMENT_ERROR,
333335
"transliterator_transliterate: \"start\" argument should be "
334336
"non-negative and not bigger than \"end\" (if defined)", 0 );
335-
RETURN_FALSE;
337+
RETVAL_FALSE;
338+
goto cleanup_object;
336339
}
337340

338341
/* end argument parsing/validation */
339342

340343
TRANSLITERATOR_METHOD_FETCH_OBJECT;
341344

342-
intl_convert_utf8_to_utf16( &ustr, &ustr_len, str, str_len,
343-
TRANSLITERATOR_ERROR_CODE_P( to ) );
344-
INTL_METHOD_CHECK_STATUS( to, "String conversion of string to UTF-16 failed" );
345+
intl_convert_utf8_to_utf16(&ustr, &ustr_len, str, str_len, TRANSLITERATOR_ERROR_CODE_P(to));
346+
INTL_METHOD_CHECK_STATUS_OR_GOTO(to, "String conversion of string to UTF-16 failed", cleanup_object);
345347

346348
/* we've started allocating resources, goto from now on */
347349

348-
if( ( start > ustr_len ) || (( limit != -1 ) && (limit > ustr_len ) ) )
350+
if( ( start > ustr_len ) || (( limit != -1 ) && (limit > ustr_len ) ) ) /* doesn't leak */
349351
{
350352
char *msg;
351353
spprintf( &msg, 0,
@@ -419,9 +421,8 @@ PHP_FUNCTION( transliterator_transliterate )
419421
RETVAL_FALSE;
420422
}
421423

422-
if (object != &tmp_object) {
423-
zval_ptr_dtor( &tmp_object );
424-
}
424+
cleanup_object:
425+
zval_ptr_dtor( &tmp_object );
425426
}
426427
/* }}} */
427428

‎ext/spl/spl_array.c

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1174,12 +1174,9 @@ PHP_METHOD(ArrayObject, __construct)
11741174
return; /* nothing to do */
11751175
}
11761176

1177-
ZEND_PARSE_PARAMETERS_START(0, 3)
1178-
Z_PARAM_OPTIONAL
1179-
Z_PARAM_ARRAY_OR_OBJECT(array)
1180-
Z_PARAM_LONG(ar_flags)
1181-
Z_PARAM_CLASS(ce_get_iterator)
1182-
ZEND_PARSE_PARAMETERS_END();
1177+
if (zend_parse_parameters(ZEND_NUM_ARGS(), "|AlC", &array, &ar_flags, &ce_get_iterator) == FAILURE) {
1178+
RETURN_THROWS();
1179+
}
11831180

11841181
intern = Z_SPLARRAY_P(object);
11851182

@@ -1205,11 +1202,9 @@ PHP_METHOD(ArrayIterator, __construct)
12051202
return; /* nothing to do */
12061203
}
12071204

1208-
ZEND_PARSE_PARAMETERS_START(0, 2)
1209-
Z_PARAM_OPTIONAL
1210-
Z_PARAM_ARRAY_OR_OBJECT(array)
1211-
Z_PARAM_LONG(ar_flags)
1212-
ZEND_PARSE_PARAMETERS_END();
1205+
if (zend_parse_parameters(ZEND_NUM_ARGS(), "|Al", &array, &ar_flags) == FAILURE) {
1206+
RETURN_THROWS();
1207+
}
12131208

12141209
intern = Z_SPLARRAY_P(object);
12151210

@@ -1284,9 +1279,9 @@ PHP_METHOD(ArrayObject, exchangeArray)
12841279
zval *object = ZEND_THIS, *array;
12851280
spl_array_object *intern = Z_SPLARRAY_P(object);
12861281

1287-
ZEND_PARSE_PARAMETERS_START(1, 1)
1288-
Z_PARAM_ARRAY_OR_OBJECT(array)
1289-
ZEND_PARSE_PARAMETERS_END();
1282+
if (zend_parse_parameters(ZEND_NUM_ARGS(), "A", &array) == FAILURE) {
1283+
RETURN_THROWS();
1284+
}
12901285

12911286
if (intern->nApplyCount > 0) {
12921287
zend_throw_error(NULL, "Modification of ArrayObject during sorting is prohibited");

‎ext/spl/spl_dllist.c

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -739,32 +739,29 @@ PHP_METHOD(SplDoublyLinkedList, offsetExists)
739739
/* {{{ Returns the value at the specified $index. */
740740
PHP_METHOD(SplDoublyLinkedList, offsetGet)
741741
{
742-
zval *zindex;
743742
zend_long index;
744743
spl_dllist_object *intern;
745744
spl_ptr_llist_element *element;
746745

747-
if (zend_parse_parameters(ZEND_NUM_ARGS(), "z", &zindex) == FAILURE) {
746+
if (zend_parse_parameters(ZEND_NUM_ARGS(), "l", &index) == FAILURE) {
748747
RETURN_THROWS();
749748
}
750749

751750
intern = Z_SPLDLLIST_P(ZEND_THIS);
752-
index = spl_offset_convert_to_long(zindex);
753751

754752
if (index < 0 || index >= intern->llist->count) {
755-
zend_throw_exception(spl_ce_OutOfRangeException, "Offset invalid or out of range", 0);
753+
zend_argument_error(spl_ce_OutOfRangeException, 1, "is out of range");
756754
RETURN_THROWS();
757755
}
758756

759757
element = spl_ptr_llist_offset(intern->llist, index, intern->flags & SPL_DLLIST_IT_LIFO);
760-
761-
if (element != NULL) {
762-
zval *value = &element->data;
763-
764-
ZVAL_COPY_DEREF(return_value, value);
765-
} else {
766-
zend_throw_exception(spl_ce_OutOfRangeException, "Offset invalid", 0);
758+
if (element == NULL) {
759+
zend_argument_error(spl_ce_OutOfRangeException, 1, "is an invalid offset");
760+
RETURN_THROWS();
767761
}
762+
763+
zval *value = &element->data;
764+
ZVAL_COPY_DEREF(return_value, value);
768765
} /* }}} */
769766

770767
/* {{{ Sets the value at the specified $index to $newval. */
@@ -790,7 +787,7 @@ PHP_METHOD(SplDoublyLinkedList, offsetSet)
790787
index = spl_offset_convert_to_long(zindex);
791788

792789
if (index < 0 || index >= intern->llist->count) {
793-
zend_throw_exception(spl_ce_OutOfRangeException, "Offset invalid or out of range", 0);
790+
zend_argument_error(spl_ce_OutOfRangeException, 1, "is out of range");
794791
RETURN_THROWS();
795792
}
796793

@@ -813,7 +810,7 @@ PHP_METHOD(SplDoublyLinkedList, offsetSet)
813810
}
814811
} else {
815812
zval_ptr_dtor(value);
816-
zend_throw_exception(spl_ce_OutOfRangeException, "Offset invalid", 0);
813+
zend_argument_error(spl_ce_OutOfRangeException, 1, "is an invalid offset");
817814
RETURN_THROWS();
818815
}
819816
}
@@ -837,7 +834,7 @@ PHP_METHOD(SplDoublyLinkedList, offsetUnset)
837834
llist = intern->llist;
838835

839836
if (index < 0 || index >= intern->llist->count) {
840-
zend_throw_exception(spl_ce_OutOfRangeException, "Offset out of range", 0);
837+
zend_argument_error(spl_ce_OutOfRangeException, 1, "is out of range");
841838
RETURN_THROWS();
842839
}
843840

@@ -878,7 +875,7 @@ PHP_METHOD(SplDoublyLinkedList, offsetUnset)
878875

879876
SPL_LLIST_DELREF(element);
880877
} else {
881-
zend_throw_exception(spl_ce_OutOfRangeException, "Offset invalid", 0);
878+
zend_argument_error(spl_ce_OutOfRangeException, 1, "is an invalid offset");
882879
RETURN_THROWS();
883880
}
884881
} /* }}} */
@@ -1243,20 +1240,19 @@ PHP_METHOD(SplDoublyLinkedList, __unserialize) {
12431240
/* {{{ Inserts a new entry before the specified $index consisting of $newval. */
12441241
PHP_METHOD(SplDoublyLinkedList, add)
12451242
{
1246-
zval *zindex, *value;
1243+
zval *value;
12471244
spl_dllist_object *intern;
12481245
spl_ptr_llist_element *element;
12491246
zend_long index;
12501247

1251-
if (zend_parse_parameters(ZEND_NUM_ARGS(), "zz", &zindex, &value) == FAILURE) {
1248+
if (zend_parse_parameters(ZEND_NUM_ARGS(), "lz", &index, &value) == FAILURE) {
12521249
RETURN_THROWS();
12531250
}
12541251

12551252
intern = Z_SPLDLLIST_P(ZEND_THIS);
1256-
index = spl_offset_convert_to_long(zindex);
12571253

12581254
if (index < 0 || index > intern->llist->count) {
1259-
zend_throw_exception(spl_ce_OutOfRangeException, "Offset invalid or out of range", 0);
1255+
zend_argument_error(spl_ce_OutOfRangeException, 1, "is out of range");
12601256
RETURN_THROWS();
12611257
}
12621258

‎ext/spl/spl_dllist.stub.php

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,8 @@
44

55
class SplDoublyLinkedList implements Iterator, Countable, ArrayAccess, Serializable
66
{
7-
/**
8-
* @param resource|string|int|float|bool $index
9-
* @return void
10-
*/
11-
public function add($index, mixed $value) {}
7+
/** @return void */
8+
public function add(int $index, mixed $value) {}
129

1310
/** @return mixed */
1411
public function pop() {}
@@ -47,19 +44,19 @@ public function getIteratorMode() {}
4744
public function offsetExists(mixed $index) {}
4845

4946
/**
50-
* @param resource|string|int|float|bool $index
47+
* @param int $index
5148
* @return mixed
5249
*/
5350
public function offsetGet($index) {}
5451

5552
/**
56-
* @param resource|string|int|float|bool|null $index
53+
* @param int|null $index
5754
* @return void
5855
*/
5956
public function offsetSet($index, mixed $value) {}
6057

6158
/**
62-
* @param resource|string|int|float|bool $index
59+
* @param int $index
6360
* @return void
6461
*/
6562
public function offsetUnset($index) {}

‎ext/spl/spl_dllist_arginfo.h

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
/* This is a generated file, edit the .stub.php file instead.
2-
* Stub hash: 58e1e222a4ef53ab086c6180445c8b79bb85baa5 */
2+
* Stub hash: 983510483588b313a0cb290db5c82e825a80be62 */
33

44
ZEND_BEGIN_ARG_INFO_EX(arginfo_class_SplDoublyLinkedList_add, 0, 0, 2)
5-
ZEND_ARG_INFO(0, index)
5+
ZEND_ARG_TYPE_INFO(0, index, IS_LONG, 0)
66
ZEND_ARG_TYPE_INFO(0, value, IS_MIXED, 0)
77
ZEND_END_ARG_INFO()
88

@@ -41,7 +41,10 @@ ZEND_BEGIN_ARG_INFO_EX(arginfo_class_SplDoublyLinkedList_offsetGet, 0, 0, 1)
4141
ZEND_ARG_INFO(0, index)
4242
ZEND_END_ARG_INFO()
4343

44-
#define arginfo_class_SplDoublyLinkedList_offsetSet arginfo_class_SplDoublyLinkedList_add
44+
ZEND_BEGIN_ARG_INFO_EX(arginfo_class_SplDoublyLinkedList_offsetSet, 0, 0, 2)
45+
ZEND_ARG_INFO(0, index)
46+
ZEND_ARG_TYPE_INFO(0, value, IS_MIXED, 0)
47+
ZEND_END_ARG_INFO()
4548

4649
#define arginfo_class_SplDoublyLinkedList_offsetUnset arginfo_class_SplDoublyLinkedList_offsetGet
4750

0 commit comments

Comments
 (0)