Skip to content

Commit 33f1cf2

Browse files
GirgiasGirgias
authored
ext/intl: Various refactoring (php#14360)
* ext/intl: Small extension cleanup * ext/intl: Normalize cloning error handling behaviour Always throw a Error exception as we cannot progress from here * ext/intl: idn.c use ValueErrors where appropriate Drive-by refactoring * ext/intl: Remove some unused headers Probably more cleanup can be done
1 parent 1ab4520 commit 33f1cf2

25 files changed

+145
-268
lines changed

‎ext/intl/breakiterator/breakiterator_class.cpp

Lines changed: 6 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -96,37 +96,22 @@ static int BreakIterator_compare_objects(zval *object1,
9696
/* {{{ clone handler for BreakIterator */
9797
static zend_object *BreakIterator_clone_obj(zend_object *object)
9898
{
99-
BreakIterator_object *bio_orig,
100-
*bio_new;
101-
zend_object *ret_val;
102-
103-
bio_orig = php_intl_breakiterator_fetch_object(object);
104-
intl_errors_reset(INTL_DATA_ERROR_P(bio_orig));
105-
106-
ret_val = BreakIterator_ce_ptr->create_object(object->ce);
107-
bio_new = php_intl_breakiterator_fetch_object(ret_val);
99+
BreakIterator_object *bio_orig = php_intl_breakiterator_fetch_object(object);
100+
zend_object *ret_val = BreakIterator_ce_ptr->create_object(object->ce);
101+
BreakIterator_object *bio_new = php_intl_breakiterator_fetch_object(ret_val);
108102

109103
zend_objects_clone_members(&bio_new->zo, &bio_orig->zo);
110104

111105
if (bio_orig->biter != NULL) {
112-
BreakIterator *new_biter;
113-
114-
new_biter = bio_orig->biter->clone();
106+
BreakIterator *new_biter = bio_orig->biter->clone();
115107
if (!new_biter) {
116-
zend_string *err_msg;
117-
intl_errors_set_code(BREAKITER_ERROR_P(bio_orig),
118-
U_MEMORY_ALLOCATION_ERROR);
119-
intl_errors_set_custom_msg(BREAKITER_ERROR_P(bio_orig),
120-
"Could not clone BreakIterator", 0);
121-
err_msg = intl_error_get_message(BREAKITER_ERROR_P(bio_orig));
122-
zend_throw_exception(NULL, ZSTR_VAL(err_msg), 0);
123-
zend_string_free(err_msg);
108+
zend_throw_error(NULL, "Failed to clone BreakIterator");
124109
} else {
125110
bio_new->biter = new_biter;
126111
ZVAL_COPY(&bio_new->text, &bio_orig->text);
127112
}
128113
} else {
129-
zend_throw_exception(NULL, "Cannot clone unconstructed BreakIterator", 0);
114+
zend_throw_error(NULL, "Cannot clone uninitialized BreakIterator");
130115
}
131116

132117
return ret_val;

‎ext/intl/calendar/calendar_class.cpp

Lines changed: 5 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -77,16 +77,9 @@ U_CFUNC void calendar_object_construct(zval *object,
7777
/* {{{ clone handler for Calendar */
7878
static zend_object *Calendar_clone_obj(zend_object *object)
7979
{
80-
Calendar_object *co_orig,
81-
*co_new;
82-
zend_object *ret_val;
83-
intl_error_reset(NULL);
84-
85-
co_orig = php_intl_calendar_fetch_object(object);
86-
intl_error_reset(INTL_DATA_ERROR_P(co_orig));
87-
88-
ret_val = Calendar_ce_ptr->create_object(object->ce);
89-
co_new = php_intl_calendar_fetch_object(ret_val);
80+
Calendar_object *co_orig = php_intl_calendar_fetch_object(object);
81+
zend_object *ret_val = Calendar_ce_ptr->create_object(object->ce);
82+
Calendar_object *co_new = php_intl_calendar_fetch_object(ret_val);
9083

9184
zend_objects_clone_members(&co_new->zo, &co_orig->zo);
9285

@@ -95,19 +88,12 @@ static zend_object *Calendar_clone_obj(zend_object *object)
9588

9689
newCalendar = co_orig->ucal->clone();
9790
if (UNEXPECTED(!newCalendar)) {
98-
zend_string *err_msg;
99-
intl_errors_set_code(CALENDAR_ERROR_P(co_orig),
100-
U_MEMORY_ALLOCATION_ERROR);
101-
intl_errors_set_custom_msg(CALENDAR_ERROR_P(co_orig),
102-
"Could not clone IntlCalendar", 0);
103-
err_msg = intl_error_get_message(CALENDAR_ERROR_P(co_orig));
104-
zend_throw_exception(NULL, ZSTR_VAL(err_msg), 0);
105-
zend_string_free(err_msg);
91+
zend_throw_error(NULL, "Failed to clone IntlCalendar");
10692
} else {
10793
co_new->ucal = newCalendar;
10894
}
10995
} else {
110-
zend_throw_exception(NULL, "Cannot clone unconstructed IntlCalendar", 0);
96+
zend_throw_error(NULL, "Cannot clone uninitialized IntlCalendar");
11197
}
11298

11399
return ret_val;

‎ext/intl/collator/collator_class.c

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -77,15 +77,6 @@ void collator_register_Collator_symbols(int module_number)
7777
Collator_handlers.offset = XtOffsetOf(Collator_object, zo);
7878
Collator_handlers.clone_obj = NULL;
7979
Collator_handlers.free_obj = Collator_objects_free;
80-
81-
/* Declare 'Collator' class properties. */
82-
if( !Collator_ce_ptr )
83-
{
84-
zend_error( E_ERROR,
85-
"Collator: attempt to create properties "
86-
"on a non-registered class." );
87-
return;
88-
}
8980
}
9081
/* }}} */
9182

‎ext/intl/converter/converter.c

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -929,8 +929,6 @@ static zend_object *php_converter_clone_object(zend_object *object) {
929929
zend_object *retval = php_converter_object_ctor(object->ce, &objval);
930930
UErrorCode error = U_ZERO_ERROR;
931931

932-
intl_errors_reset(&oldobj->error);
933-
934932
#if U_ICU_VERSION_MAJOR_NUM > 70
935933
objval->src = ucnv_clone(oldobj->src, &error);
936934
#else
@@ -944,14 +942,9 @@ static zend_object *php_converter_clone_object(zend_object *object) {
944942
objval->dest = ucnv_safeClone(oldobj->dest, NULL, NULL, &error);
945943
#endif
946944
}
947-
if (U_FAILURE(error)) {
948-
zend_string *err_msg;
949-
THROW_UFAILURE(oldobj, "ucnv_safeClone", error);
950-
951-
err_msg = intl_error_get_message(&oldobj->error);
952-
zend_throw_exception(NULL, ZSTR_VAL(err_msg), 0);
953-
zend_string_release_ex(err_msg, 0);
954945

946+
if (U_FAILURE(error)) {
947+
zend_throw_error(NULL, "Failed to clone UConverter");
955948
return retval;
956949
}
957950

‎ext/intl/dateformat/dateformat_class.c

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -64,27 +64,23 @@ zend_object *IntlDateFormatter_object_create(zend_class_entry *ce)
6464
/* {{{ IntlDateFormatter_object_clone */
6565
zend_object *IntlDateFormatter_object_clone(zend_object *object)
6666
{
67-
IntlDateFormatter_object *dfo, *new_dfo;
68-
zend_object *new_obj;
67+
IntlDateFormatter_object *dfo = php_intl_dateformatter_fetch_object(object);
68+
zend_object *new_obj = IntlDateFormatter_ce_ptr->create_object(object->ce);
69+
IntlDateFormatter_object *new_dfo = php_intl_dateformatter_fetch_object(new_obj);
6970

70-
dfo = php_intl_dateformatter_fetch_object(object);
71-
intl_error_reset(INTL_DATA_ERROR_P(dfo));
72-
73-
new_obj = IntlDateFormatter_ce_ptr->create_object(object->ce);
74-
new_dfo = php_intl_dateformatter_fetch_object(new_obj);
7571
/* clone standard parts */
7672
zend_objects_clone_members(&new_dfo->zo, &dfo->zo);
73+
7774
/* clone formatter object */
78-
if (dfo->datef_data.udatf != NULL) {
79-
DATE_FORMAT_OBJECT(new_dfo) = udat_clone(DATE_FORMAT_OBJECT(dfo), &INTL_DATA_ERROR_CODE(dfo));
80-
if (U_FAILURE(INTL_DATA_ERROR_CODE(dfo))) {
81-
/* set up error in case error handler is interested */
82-
intl_errors_set(INTL_DATA_ERROR_P(dfo), INTL_DATA_ERROR_CODE(dfo),
83-
"Failed to clone IntlDateFormatter object", 0 );
84-
zend_throw_exception(NULL, "Failed to clone IntlDateFormatter object", 0);
75+
if (DATE_FORMAT_OBJECT(dfo) != NULL) {
76+
UErrorCode error = U_ZERO_ERROR;
77+
DATE_FORMAT_OBJECT(new_dfo) = udat_clone(DATE_FORMAT_OBJECT(dfo), &error);
78+
79+
if (U_FAILURE(error)) {
80+
zend_throw_error(NULL, "Failed to clone IntlDateFormatter");
8581
}
8682
} else {
87-
zend_throw_exception(NULL, "Cannot clone unconstructed IntlDateFormatter", 0);
83+
zend_throw_error(NULL, "Cannot clone uninitialized IntlDateFormatter");
8884
}
8985
return new_obj;
9086
}

‎ext/intl/dateformat/datepatterngenerator_class.cpp

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -36,32 +36,21 @@ zend_object_handlers IntlDatePatternGenerator_handlers;
3636

3737
static zend_object *IntlDatePatternGenerator_object_clone(zend_object *object)
3838
{
39-
intl_error_reset(NULL);
40-
4139
IntlDatePatternGenerator_object *dtpgo_orig = php_intl_datepatterngenerator_fetch_object(object);
42-
intl_error_reset(DTPATTERNGEN_ERROR_P(dtpgo_orig));
43-
44-
zend_object *ret_val = IntlDatePatternGenerator_ce_ptr->create_object(object->ce);
45-
IntlDatePatternGenerator_object *dtpgo_new = php_intl_datepatterngenerator_fetch_object(ret_val);
40+
zend_object *ret_val = IntlDatePatternGenerator_ce_ptr->create_object(object->ce);
41+
IntlDatePatternGenerator_object *dtpgo_new = php_intl_datepatterngenerator_fetch_object(ret_val);
4642

4743
zend_objects_clone_members(&dtpgo_new->zo, &dtpgo_orig->zo);
4844

4945
if (dtpgo_orig->dtpg != NULL) {
5046
DateTimePatternGenerator *newDtpg = dtpgo_orig->dtpg->clone();
5147
if (!newDtpg) {
52-
zend_string *err_msg;
53-
intl_errors_set_code(DTPATTERNGEN_ERROR_P(dtpgo_orig),
54-
U_MEMORY_ALLOCATION_ERROR);
55-
intl_errors_set_custom_msg(DTPATTERNGEN_ERROR_P(dtpgo_orig),
56-
"Could not clone IntlDatePatternGenerator", 0);
57-
err_msg = intl_error_get_message(DTPATTERNGEN_ERROR_P(dtpgo_orig));
58-
zend_throw_exception(NULL, ZSTR_VAL(err_msg), 0);
59-
zend_string_free(err_msg);
48+
zend_throw_error(NULL, "Failed to clone IntlDatePatternGenerator");
6049
} else {
6150
dtpgo_new->dtpg = newDtpg;
6251
}
6352
} else {
64-
zend_throw_exception(NULL, "Cannot clone unconstructed IntlDatePatternGenerator", 0);
53+
zend_throw_error(NULL, "Cannot clone uninitialized IntlDatePatternGenerator");
6554
}
6655

6756
return ret_val;

‎ext/intl/formatter/formatter_class.c

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -58,28 +58,22 @@ zend_object *NumberFormatter_object_create(zend_class_entry *ce)
5858
/* {{{ NumberFormatter_object_clone */
5959
zend_object *NumberFormatter_object_clone(zend_object *object)
6060
{
61-
NumberFormatter_object *nfo, *new_nfo;
62-
zend_object *new_obj;
61+
NumberFormatter_object *nfo = php_intl_number_format_fetch_object(object);
62+
zend_object *new_obj = NumberFormatter_ce_ptr->create_object(object->ce);
63+
NumberFormatter_object *new_nfo = php_intl_number_format_fetch_object(new_obj);
6364

64-
nfo = php_intl_number_format_fetch_object(object);
65-
intl_error_reset(INTL_DATA_ERROR_P(nfo));
66-
67-
new_obj = NumberFormatter_ce_ptr->create_object(object->ce);
68-
new_nfo = php_intl_number_format_fetch_object(new_obj);
6965
/* clone standard parts */
7066
zend_objects_clone_members(&new_nfo->zo, &nfo->zo);
67+
7168
/* clone formatter object. It may fail, the destruction code must handle this case */
7269
if (FORMATTER_OBJECT(nfo) != NULL) {
73-
FORMATTER_OBJECT(new_nfo) = unum_clone(FORMATTER_OBJECT(nfo),
74-
&INTL_DATA_ERROR_CODE(nfo));
75-
if (U_FAILURE(INTL_DATA_ERROR_CODE(nfo))) {
76-
/* set up error in case error handler is interested */
77-
intl_errors_set(INTL_DATA_ERROR_P(nfo), INTL_DATA_ERROR_CODE(nfo),
78-
"Failed to clone NumberFormatter object", 0);
79-
zend_throw_exception(NULL, "Failed to clone NumberFormatter object", 0);
70+
UErrorCode error = U_ZERO_ERROR;
71+
FORMATTER_OBJECT(new_nfo) = unum_clone(FORMATTER_OBJECT(nfo), &error);
72+
if (U_FAILURE(error)) {
73+
zend_throw_error(NULL, "Failed to clone NumberFormatter");
8074
}
8175
} else {
82-
zend_throw_exception(NULL, "Cannot clone unconstructed NumberFormatter", 0);
76+
zend_throw_error(NULL, "Cannot clone uninitialized NumberFormatter");
8377
}
8478
return new_obj;
8579
}

‎ext/intl/grapheme/grapheme_util.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,6 @@
2828
#include <unicode/ubrk.h>
2929
#include <unicode/usearch.h>
3030

31-
#include "ext/standard/php_string.h"
32-
3331
ZEND_EXTERN_MODULE_GLOBALS( intl )
3432

3533
/* }}} */

‎ext/intl/idn/idn.c

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ enum {
3737
};
3838

3939
/* like INTL_CHECK_STATUS, but as a function and varying the name of the func */
40-
static int php_intl_idn_check_status(UErrorCode err, const char *msg)
40+
static zend_result php_intl_idn_check_status(UErrorCode err, const char *msg)
4141
{
4242
intl_error_set_code(NULL, err);
4343
if (U_FAILURE(err)) {
@@ -53,11 +53,6 @@ static int php_intl_idn_check_status(UErrorCode err, const char *msg)
5353
return SUCCESS;
5454
}
5555

56-
static inline void php_intl_bad_args(const char *msg)
57-
{
58-
php_intl_idn_check_status(U_ILLEGAL_ARGUMENT_ERROR, msg);
59-
}
60-
6156
static void php_intl_idn_to_46(INTERNAL_FUNCTION_PARAMETERS,
6257
const zend_string *domain, uint32_t option, int mode, zval *idna_info)
6358
{
@@ -128,18 +123,17 @@ static void php_intl_idn_handoff(INTERNAL_FUNCTION_PARAMETERS, int mode)
128123
RETURN_THROWS();
129124
}
130125

131-
if (variant != INTL_IDN_VARIANT_UTS46) {
132-
php_intl_bad_args("invalid variant, must be INTL_IDNA_VARIANT_UTS46");
133-
RETURN_FALSE;
134-
}
135-
136-
if (ZSTR_LEN(domain) < 1) {
137-
php_intl_bad_args("empty domain name");
138-
RETURN_FALSE;
126+
if (ZSTR_LEN(domain) == 0) {
127+
zend_argument_value_error(1, "cannot be empty");
128+
RETURN_THROWS();
139129
}
140130
if (ZSTR_LEN(domain) > INT32_MAX - 1) {
141-
php_intl_bad_args("domain name too large");
142-
RETURN_FALSE;
131+
zend_argument_value_error(1, "must be less than " PRId32 " bytes", INT32_MAX);
132+
RETURN_THROWS();
133+
}
134+
if (variant != INTL_IDN_VARIANT_UTS46) {
135+
zend_argument_value_error(2, "must be INTL_IDNA_VARIANT_UTS46");
136+
RETURN_THROWS();
143137
}
144138
/* don't check options; it wasn't checked before */
145139

‎ext/intl/msgformat/msgformat_class.c

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -56,29 +56,23 @@ zend_object *MessageFormatter_object_create(zend_class_entry *ce)
5656
/* {{{ MessageFormatter_object_clone */
5757
zend_object *MessageFormatter_object_clone(zend_object *object)
5858
{
59-
MessageFormatter_object *mfo, *new_mfo;
60-
zend_object *new_obj;
59+
MessageFormatter_object *mfo = php_intl_messageformatter_fetch_object(object);
60+
zend_object *new_obj = MessageFormatter_ce_ptr->create_object(object->ce);
61+
MessageFormatter_object *new_mfo = php_intl_messageformatter_fetch_object(new_obj);
6162

62-
mfo = php_intl_messageformatter_fetch_object(object);
63-
intl_error_reset(INTL_DATA_ERROR_P(mfo));
64-
65-
new_obj = MessageFormatter_ce_ptr->create_object(object->ce);
66-
new_mfo = php_intl_messageformatter_fetch_object(new_obj);
6763
/* clone standard parts */
6864
zend_objects_clone_members(&new_mfo->zo, &mfo->zo);
6965

7066
/* clone formatter object */
7167
if (MSG_FORMAT_OBJECT(mfo) != NULL) {
72-
MSG_FORMAT_OBJECT(new_mfo) = umsg_clone(MSG_FORMAT_OBJECT(mfo),
73-
&INTL_DATA_ERROR_CODE(mfo));
68+
UErrorCode error = U_ZERO_ERROR;
69+
MSG_FORMAT_OBJECT(new_mfo) = umsg_clone(MSG_FORMAT_OBJECT(mfo), &error);
7470

75-
if (U_FAILURE(INTL_DATA_ERROR_CODE(mfo))) {
76-
intl_errors_set(INTL_DATA_ERROR_P(mfo), INTL_DATA_ERROR_CODE(mfo),
77-
"Failed to clone MessageFormatter object", 0);
78-
zend_throw_exception_ex(NULL, 0, "Failed to clone MessageFormatter object");
71+
if (U_FAILURE(error)) {
72+
zend_throw_error(NULL, "Failed to clone MessageFormatter");
7973
}
8074
} else {
81-
zend_throw_exception_ex(NULL, 0, "Cannot clone unconstructed MessageFormatter");
75+
zend_throw_error(NULL, "Cannot clone uninitialized MessageFormatter");
8276
}
8377
return new_obj;
8478
}

‎ext/intl/resourcebundle/resourcebundle_iterator.c

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -148,13 +148,14 @@ static const zend_object_iterator_funcs resourcebundle_iterator_funcs = {
148148
/* {{{ resourcebundle_get_iterator */
149149
zend_object_iterator *resourcebundle_get_iterator( zend_class_entry *ce, zval *object, int byref )
150150
{
151-
ResourceBundle_object *rb = Z_INTL_RESOURCEBUNDLE_P(object );
152-
ResourceBundle_iterator *iterator = emalloc( sizeof( ResourceBundle_iterator ) );
153-
154151
if (byref) {
155-
php_error( E_ERROR, "ResourceBundle does not support writable iterators" );
152+
zend_throw_error(NULL, "An iterator cannot be used with foreach by reference");
153+
return NULL;
156154
}
157155

156+
ResourceBundle_object *rb = Z_INTL_RESOURCEBUNDLE_P(object );
157+
ResourceBundle_iterator *iterator = emalloc( sizeof( ResourceBundle_iterator ) );
158+
158159
zend_iterator_init(&iterator->intern);
159160
Z_ADDREF_P(object);
160161
ZVAL_OBJ(&iterator->intern.data, Z_OBJ_P(object));

‎ext/intl/spoofchecker/spoofchecker_class.c

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -61,24 +61,25 @@ zend_object *Spoofchecker_object_create(zend_class_entry *ce)
6161

6262
static zend_object *spoofchecker_clone_obj(zend_object *object) /* {{{ */
6363
{
64-
zend_object *new_obj_val;
65-
Spoofchecker_object *sfo, *new_sfo;
66-
67-
sfo = php_intl_spoofchecker_fetch_object(object);
68-
intl_error_reset(SPOOFCHECKER_ERROR_P(sfo));
69-
70-
new_obj_val = Spoofchecker_ce_ptr->create_object(object->ce);
71-
new_sfo = php_intl_spoofchecker_fetch_object(new_obj_val);
72-
/* clone standard parts */
73-
zend_objects_clone_members(&new_sfo->zo, &sfo->zo);
74-
/* clone internal object */
75-
new_sfo->uspoof = uspoof_clone(sfo->uspoof, SPOOFCHECKER_ERROR_CODE_P(new_sfo));
76-
if(U_FAILURE(SPOOFCHECKER_ERROR_CODE(new_sfo))) {
77-
/* set up error in case error handler is interested */
78-
intl_error_set( NULL, SPOOFCHECKER_ERROR_CODE(new_sfo), "Failed to clone SpoofChecker object", 0 );
79-
Spoofchecker_objects_free(&new_sfo->zo); /* free new object */
80-
zend_error_noreturn(E_ERROR, "Failed to clone SpoofChecker object");
64+
Spoofchecker_object *spoofchecker_orig = php_intl_spoofchecker_fetch_object(object);
65+
zend_object *new_obj_val = Spoofchecker_ce_ptr->create_object(object->ce);
66+
Spoofchecker_object *spoofchecker_new = php_intl_spoofchecker_fetch_object(new_obj_val);
67+
68+
zend_objects_clone_members(&spoofchecker_new->zo, &spoofchecker_orig->zo);
69+
70+
if (spoofchecker_orig->uspoof != NULL) {
71+
/* guaranteed to return NULL if it fails */
72+
UErrorCode error = U_ZERO_ERROR;
73+
spoofchecker_new->uspoof = uspoof_clone(spoofchecker_orig->uspoof, &error);
74+
if (U_FAILURE(error)) {
75+
/* free new object */
76+
Spoofchecker_objects_free(&spoofchecker_new->zo);
77+
zend_throw_error(NULL, "Failed to clone SpoofChecker");
78+
}
79+
} else {
80+
zend_throw_error(NULL, "Cannot clone uninitialized SpoofChecker");
8181
}
82+
8283
return new_obj_val;
8384
}
8485
/* }}} */

0 commit comments

Comments
 (0)