Skip to content

Commit a985c84

Browse files
GirgiasGirgias
committed
refactor zend_parse_arg_class() so that it can be reused
And stop throwing the exceptions directly Means we restored back the original error message behaviour that's possibly not ideal
1 parent b712cda commit a985c84

4 files changed

Lines changed: 113 additions & 86 deletions

File tree

‎Zend/zend_API.c‎

Lines changed: 102 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -225,48 +225,6 @@ ZEND_API ZEND_COLD void ZEND_FASTCALL zend_wrong_parameters_count_error(uint32_t
225225
}
226226
/* }}} */
227227

228-
ZEND_API ZEND_COLD void ZEND_FASTCALL zend_wrong_parameter_error(zpp_error error_code, uint32_t num, char *name, zend_expected_type expected_type, const zval *arg) /* {{{ */
229-
{
230-
switch (error_code) {
231-
case ZPP_ERROR_WRONG_CALLBACK:
232-
zend_wrong_callback_error(num, name);
233-
break;
234-
case ZPP_ERROR_WRONG_CALLBACK_OR_NULL:
235-
zend_wrong_callback_or_null_error(num, name);
236-
break;
237-
case ZPP_ERROR_WRONG_CLASS:
238-
zend_wrong_parameter_class_error(num, name, arg);
239-
break;
240-
case ZPP_ERROR_WRONG_CLASS_OR_NULL:
241-
zend_wrong_parameter_class_or_null_error(num, name, arg);
242-
break;
243-
case ZPP_ERROR_WRONG_CLASS_OR_STRING:
244-
zend_wrong_parameter_class_or_string_error(num, name, arg);
245-
break;
246-
case ZPP_ERROR_WRONG_CLASS_OR_STRING_OR_NULL:
247-
zend_wrong_parameter_class_or_string_or_null_error(num, name, arg);
248-
break;
249-
case ZPP_ERROR_WRONG_CLASS_OR_LONG:
250-
zend_wrong_parameter_class_or_long_error(num, name, arg);
251-
break;
252-
case ZPP_ERROR_WRONG_CLASS_OR_LONG_OR_NULL:
253-
zend_wrong_parameter_class_or_long_or_null_error(num, name, arg);
254-
break;
255-
case ZPP_ERROR_WRONG_ARG:
256-
zend_wrong_parameter_type_error(num, expected_type, arg);
257-
break;
258-
case ZPP_ERROR_UNEXPECTED_EXTRA_NAMED:
259-
zend_unexpected_extra_named_error();
260-
break;
261-
case ZPP_ERROR_FAILURE:
262-
ZEND_ASSERT(EG(exception) && "Should have produced an error already");
263-
break;
264-
case ZPP_ERROR_OK:
265-
ZEND_UNREACHABLE();
266-
}
267-
}
268-
/* }}} */
269-
270228
ZEND_API ZEND_COLD void ZEND_FASTCALL zend_wrong_parameter_type_error(uint32_t num, zend_expected_type expected_type, const zval *arg) /* {{{ */
271229
{
272230
static const char * const expected_error[] = {
@@ -348,6 +306,42 @@ ZEND_API ZEND_COLD void ZEND_FASTCALL zend_wrong_parameter_class_or_string_or_nu
348306
}
349307
/* }}} */
350308

309+
static ZEND_COLD void ZEND_FASTCALL zend_wrong_class_name_error(uint32_t num, char *name, const zval *arg)
310+
{
311+
if (EG(exception)) {
312+
return;
313+
}
314+
315+
if (Z_TYPE_P(arg) != IS_STRING) {
316+
zend_wrong_parameter_type_error(num, Z_EXPECTED_STRING, arg);
317+
return;
318+
}
319+
320+
if (name) {
321+
zend_argument_type_error(num, "must be a class name derived from %s, %s given", name, Z_STRVAL_P(arg));
322+
} else {
323+
zend_argument_type_error(num, "must be a valid class name, %s given", Z_STRVAL_P(arg));
324+
}
325+
}
326+
327+
static ZEND_COLD void ZEND_FASTCALL zend_wrong_class_name_or_null_error(uint32_t num, char *name, const zval *arg)
328+
{
329+
if (EG(exception)) {
330+
return;
331+
}
332+
333+
if (Z_TYPE_P(arg) != IS_STRING) {
334+
zend_wrong_parameter_type_error(num, Z_EXPECTED_STRING_OR_NULL, arg);
335+
return;
336+
}
337+
338+
if (name) {
339+
zend_argument_type_error(num, "must be a class name derived from %s or null, %s given", name, Z_STRVAL_P(arg));
340+
} else {
341+
zend_argument_type_error(num, "must be a valid class name or null, %s given", Z_STRVAL_P(arg));
342+
}
343+
}
344+
351345
ZEND_API ZEND_COLD void ZEND_FASTCALL zend_wrong_callback_error(uint32_t num, char *error) /* {{{ */
352346
{
353347
if (!EG(exception)) {
@@ -366,6 +360,54 @@ ZEND_API ZEND_COLD void ZEND_FASTCALL zend_wrong_callback_or_null_error(uint32_t
366360
}
367361
/* }}} */
368362

363+
ZEND_API ZEND_COLD void ZEND_FASTCALL zend_wrong_parameter_error(zpp_error error_code, uint32_t num, char *name, zend_expected_type expected_type, const zval *arg) /* {{{ */
364+
{
365+
switch (error_code) {
366+
case ZPP_ERROR_WRONG_CALLBACK:
367+
zend_wrong_callback_error(num, name);
368+
break;
369+
case ZPP_ERROR_WRONG_CALLBACK_OR_NULL:
370+
zend_wrong_callback_or_null_error(num, name);
371+
break;
372+
case ZPP_ERROR_WRONG_CLASS_NAME:
373+
zend_wrong_class_name_error(num, name, arg);
374+
break;
375+
case ZPP_ERROR_WRONG_CLASS_NAME_OR_NULL:
376+
zend_wrong_class_name_or_null_error(num, name, arg);
377+
break;
378+
case ZPP_ERROR_WRONG_CLASS:
379+
zend_wrong_parameter_class_error(num, name, arg);
380+
break;
381+
case ZPP_ERROR_WRONG_CLASS_OR_NULL:
382+
zend_wrong_parameter_class_or_null_error(num, name, arg);
383+
break;
384+
case ZPP_ERROR_WRONG_CLASS_OR_STRING:
385+
zend_wrong_parameter_class_or_string_error(num, name, arg);
386+
break;
387+
case ZPP_ERROR_WRONG_CLASS_OR_STRING_OR_NULL:
388+
zend_wrong_parameter_class_or_string_or_null_error(num, name, arg);
389+
break;
390+
case ZPP_ERROR_WRONG_CLASS_OR_LONG:
391+
zend_wrong_parameter_class_or_long_error(num, name, arg);
392+
break;
393+
case ZPP_ERROR_WRONG_CLASS_OR_LONG_OR_NULL:
394+
zend_wrong_parameter_class_or_long_or_null_error(num, name, arg);
395+
break;
396+
case ZPP_ERROR_WRONG_ARG:
397+
zend_wrong_parameter_type_error(num, expected_type, arg);
398+
break;
399+
case ZPP_ERROR_UNEXPECTED_EXTRA_NAMED:
400+
zend_unexpected_extra_named_error();
401+
break;
402+
case ZPP_ERROR_FAILURE:
403+
ZEND_ASSERT(EG(exception) && "Should have produced an error already");
404+
break;
405+
case ZPP_ERROR_OK:
406+
ZEND_UNREACHABLE();
407+
}
408+
}
409+
/* }}} */
410+
369411
ZEND_API ZEND_COLD void ZEND_FASTCALL zend_unexpected_extra_named_error(void)
370412
{
371413
const char *space;
@@ -484,34 +526,30 @@ ZEND_API ZEND_COLD void zend_class_redeclaration_error(int type, const zend_clas
484526
zend_class_redeclaration_error_ex(type, old_ce->name, old_ce);
485527
}
486528

487-
ZEND_API bool ZEND_FASTCALL zend_parse_arg_class(zval *arg, zend_class_entry **pce, uint32_t num, bool check_null) /* {{{ */
529+
ZEND_API ZEND_FASTCALL bool zend_parse_arg_class(zval *arg, zend_class_entry **pce, uint32_t num, bool check_null)
488530
{
489531
const zend_class_entry *ce_base = *pce;
490532

491533
if (check_null && Z_TYPE_P(arg) == IS_NULL) {
492534
*pce = NULL;
493-
return 1;
535+
return true;
494536
}
495537
zend_string *class_name;
496538
if (!zend_parse_arg_str(arg, &class_name, check_null, num)) {
497539
*pce = NULL;
498-
zend_wrong_parameter_error(ZPP_ERROR_WRONG_ARG, num, NULL, check_null ? Z_EXPECTED_STRING_OR_NULL : Z_EXPECTED_STRING, arg);
499-
return 0;
540+
return false;
500541
}
501542

502543
*pce = zend_lookup_class(class_name);
503544
if (!*pce) {
504-
zend_argument_type_error(num, "must be a valid class name, %s given", ZSTR_VAL(class_name));
505-
return 0;
545+
return false;
506546
}
507547
if (ce_base && !instanceof_function(*pce, ce_base)) {
508-
zend_argument_type_error(num, "must be a class name derived from %s, %s given", ZSTR_VAL(ce_base->name), ZSTR_VAL(class_name));
509548
*pce = NULL;
510-
return 0;
549+
return false;
511550
}
512-
return 1;
551+
return true;
513552
}
514-
/* }}} */
515553

516554
static ZEND_COLD bool zend_null_arg_deprecated(const char *fallback_type, uint32_t arg_num) {
517555
const zend_function *func = zend_active_function();
@@ -1005,30 +1043,10 @@ static zend_expected_type zend_parse_arg_impl(zval *arg, va_list *va, const char
10051043
zend_class_entry **pce = va_arg(*va, zend_class_entry **);
10061044
const zend_class_entry *ce_base = *pce;
10071045

1008-
if (check_null && Z_TYPE_P(arg) == IS_NULL) {
1009-
*pce = NULL;
1010-
break;
1011-
}
1012-
1013-
zend_string *class_name = NULL;
1014-
if (!zend_parse_arg_str(arg, &class_name, check_null, arg_num)) {
1015-
*pce = NULL;
1016-
return check_null ? Z_EXPECTED_STRING_OR_NULL : Z_EXPECTED_STRING;
1017-
}
1018-
1019-
*pce = zend_lookup_class(class_name);
1020-
if (!*pce) {
1021-
zend_spprintf(error, 0, "must be a valid class name%s, %s given",
1022-
check_null ? " or null" : "", Z_STRVAL_P(arg));
1023-
return check_null ? Z_EXPECTED_OBJECT_OR_CLASS_NAME_OR_NULL : Z_EXPECTED_OBJECT_OR_CLASS_NAME;
1024-
}
1025-
if (ce_base && !instanceof_function(*pce, ce_base)) {
1026-
zend_spprintf(error, 0, "must be a class name derived from %s%s, %s given",
1027-
ZSTR_VAL(ce_base->name), check_null ? " or null" : "", Z_STRVAL_P(arg));
1028-
*pce = NULL;
1029-
return check_null ? Z_EXPECTED_OBJECT_OR_CLASS_NAME_OR_NULL : Z_EXPECTED_OBJECT_OR_CLASS_NAME;
1046+
*error = *pce ? ZSTR_VAL(ce_base->name) : NULL;
1047+
if (!zend_parse_arg_class(arg, pce, arg_num, check_null)) {
1048+
return check_null ? Z_EXPECTED_CLASS_NAME_OR_NULL : Z_EXPECTED_CLASS_NAME;
10301049
}
1031-
break;
10321050

10331051
}
10341052
break;
@@ -1107,19 +1125,22 @@ static zend_result zend_parse_arg(uint32_t arg_num, zval *arg, va_list *va, cons
11071125
/* error is freed by zend_wrong_callback_or_null_error() */
11081126
zend_wrong_callback_or_null_error(arg_num, error);
11091127
break;
1110-
case Z_EXPECTED_OBJECT_OR_CLASS_NAME:
1111-
case Z_EXPECTED_OBJECT_OR_CLASS_NAME_OR_NULL:
1112-
zend_argument_type_error(arg_num, "%s", error);
1113-
efree(error);
1128+
case Z_EXPECTED_CLASS_NAME:
1129+
/* DO NOT FREE error: it's a pointer to ZSTR_VAL(ce->name) */
1130+
zend_wrong_class_name_error(arg_num, error, arg);
1131+
break;
1132+
case Z_EXPECTED_CLASS_NAME_OR_NULL:
1133+
/* DO NOT FREE error: it's a pointer to ZSTR_VAL(ce->name) */
1134+
zend_wrong_class_name_or_null_error(arg_num, error, arg);
11141135
break;
11151136
default:
11161137
ZEND_UNREACHABLE();
11171138
}
11181139
}
11191140
zend_wrong_parameter_type_error(arg_num, expected_type, arg);
11201141
} else if (error
1121-
/* DO NOT FREE error when it's a pointer to ZSTR_VAL(ce->name) */
1122-
&& expected_type != Z_EXPECTED_OBJECT && expected_type != Z_EXPECTED_OBJECT_OR_NULL) {
1142+
/* Only free error if it's a callable expected type, as otherwise it's a pointer to ZSTR_VAL(ce->name) */
1143+
&& (expected_type == Z_EXPECTED_FUNC || expected_type == Z_EXPECTED_FUNC_OR_NULL)) {
11231144
efree(error);
11241145
}
11251146

‎Zend/zend_API.h‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1577,6 +1577,8 @@ static zend_always_inline zval *zend_try_array_init(zval *zv)
15771577
_(Z_EXPECTED_OBJECT_OR_CLASS_NAME_OR_NULL, "an object, a valid class name, or null") \
15781578
_(Z_EXPECTED_OBJECT_OR_STRING, "of type object|string") \
15791579
_(Z_EXPECTED_OBJECT_OR_STRING_OR_NULL, "of type object|string|null") \
1580+
_(Z_EXPECTED_CLASS_NAME, "a valid class name") \
1581+
_(Z_EXPECTED_CLASS_NAME_OR_NULL, "a valid class name or null") \
15801582

15811583
#define Z_EXPECTED_TYPE
15821584

@@ -1592,6 +1594,9 @@ C23_ENUM(zpp_error, uint8_t) {
15921594
ZPP_ERROR_OK,
15931595
ZPP_ERROR_FAILURE,
15941596
ZPP_ERROR_WRONG_CALLBACK,
1597+
ZPP_ERROR_WRONG_CALLBACK_OR_NULL,
1598+
ZPP_ERROR_WRONG_CLASS_NAME,
1599+
ZPP_ERROR_WRONG_CLASS_NAME_OR_NULL,
15951600
ZPP_ERROR_WRONG_CLASS,
15961601
ZPP_ERROR_WRONG_CLASS_OR_NULL,
15971602
ZPP_ERROR_WRONG_CLASS_OR_STRING,
@@ -1600,7 +1605,6 @@ C23_ENUM(zpp_error, uint8_t) {
16001605
ZPP_ERROR_WRONG_CLASS_OR_LONG_OR_NULL,
16011606
ZPP_ERROR_WRONG_ARG,
16021607
ZPP_ERROR_UNEXPECTED_EXTRA_NAMED,
1603-
ZPP_ERROR_WRONG_CALLBACK_OR_NULL,
16041608
};
16051609

16061610
ZEND_API ZEND_COLD void ZEND_FASTCALL zend_wrong_parameters_none_error(void);
@@ -1773,8 +1777,10 @@ ZEND_API ZEND_COLD void zend_class_redeclaration_error_ex(int type, zend_string
17731777
/* old "C" */
17741778
#define Z_PARAM_CLASS_EX(dest, check_null, deref) \
17751779
Z_PARAM_PROLOGUE(deref, 0); \
1780+
_error = dest ? ZSTR_VAL((dest)->name) : NULL; \
17761781
if (UNEXPECTED(!zend_parse_arg_class(_arg, &dest, _i, check_null))) { \
1777-
_error_code = ZPP_ERROR_FAILURE; \
1782+
_expected_type = check_null ? Z_EXPECTED_CLASS_NAME_OR_NULL : Z_EXPECTED_CLASS_NAME; \
1783+
_error_code = check_null ? ZPP_ERROR_WRONG_CLASS_NAME_OR_NULL : ZPP_ERROR_WRONG_CLASS_NAME; \
17781784
break; \
17791785
}
17801786

‎ext/spl/tests/ArrayObject/arrayObject___construct_error1.phpt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,4 @@ try {
2222
Bad iterator type:
2323
ArrayObject::__construct(): Argument #3 ($iteratorClass) must be a class name derived from ArrayIterator, Exception given(6)
2424
Non-existent class:
25-
ArrayObject::__construct(): Argument #3 ($iteratorClass) must be a valid class name, nonExistentClassName given(13)
25+
ArrayObject::__construct(): Argument #3 ($iteratorClass) must be a class name derived from ArrayIterator, nonExistentClassName given(13)

‎ext/spl/tests/ArrayObject/arrayObject_setIteratorClass_error1.phpt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ try {
4343

4444
?>
4545
--EXPECT--
46-
string(112) "ArrayObject::setIteratorClass(): Argument #1 ($iteratorClass) must be a valid class name, nonExistentClass given"
46+
string(133) "ArrayObject::setIteratorClass(): Argument #1 ($iteratorClass) must be a class name derived from ArrayIterator, nonExistentClass given"
4747
string(125) "ArrayObject::setIteratorClass(): Argument #1 ($iteratorClass) must be a class name derived from ArrayIterator, stdClass given"
48-
string(107) "ArrayObject::__construct(): Argument #3 ($iteratorClass) must be a valid class name, nonExistentClass given"
48+
string(128) "ArrayObject::__construct(): Argument #3 ($iteratorClass) must be a class name derived from ArrayIterator, nonExistentClass given"
4949
string(120) "ArrayObject::__construct(): Argument #3 ($iteratorClass) must be a class name derived from ArrayIterator, stdClass given"

0 commit comments

Comments
 (0)