Skip to content

Commit 54b58d5

Browse files
authored
ext/intl: Refactor global error resets (#22990)
Introduce PHP_INTL_FUNCTION_WITH_ERROR_RESET macro to standardize and automate the global Intl error state reset at the start of every userland PHP_FUNCTION. Also change the IC_METHOD function to automatically reset error state in the start of the defined function. The macro wraps the function entry point, performs intl_error_reset(NULL), and delegates the actual logic to a separate _impl function. This reduces boilerplate and prevents bugs caused by missing error resets, as seen in GH-22931 and GH-22500. This refactor applies to all functions with a manual reset call and does not affect internal helper functions that handle errors independently.
1 parent ed34ad4 commit 54b58d5

9 files changed

Lines changed: 61 additions & 99 deletions

File tree

ext/intl/ERROR_CONVENTIONS.md

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -93,13 +93,21 @@ void intl_error_reset(NULL); /* reset global error */
9393
void intl_errors_reset(intl_error* err); /* reset global and object error */
9494
```
9595

96-
In practice, `intl_errors_reset()` is not used because most classes have also
97-
plain functions mapped to the same internal functions as their instance methods.
98-
Fetching of the object is done with `zend_parse_method_parameters()` instead of
99-
directly using `getThis()`. Therefore, no reference to object is obtained until
100-
the arguments are fully parsed. Without a reference to the object, there's no
101-
way to reset the object's internal error code. Instead, resetting of the
102-
object's internal error code is done upon fetching the object from its zval.
96+
Procedural functions that reset the global error should normally use
97+
`PHP_INTL_FUNCTION_WITH_ERROR_RESET(name)`, which resets the global error before
98+
entering the implementation body.
99+
Class-specific method helper macros may use the same pattern when all methods
100+
covered by the macro reset the global error.
101+
102+
`intl_errors_reset()` may be used directly when the object is available before
103+
argument parsing, for example in methods that only operate on `ZEND_THIS()`.
104+
For classes that have plain functions mapped to the same internal functions as
105+
their instance methods, fetching of the object is done with
106+
`zend_parse_method_parameters()` instead of directly using `getThis()`.
107+
Therefore, no reference to object is obtained until the arguments are fully
108+
parsed. Without a reference to the object, there's no way to reset the object's
109+
internal error code. Instead, resetting of the object's internal error code is
110+
done upon fetching the object from its zval.
103111

104112
Example:
105113

ext/intl/calendar/calendar_methods.cpp

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -70,14 +70,13 @@ U_CFUNC PHP_METHOD(IntlCalendar, __construct)
7070
0 );
7171
}
7272

73-
U_CFUNC PHP_FUNCTION(intlcal_create_instance)
73+
PHP_INTL_FUNCTION_WITH_ERROR_RESET(intlcal_create_instance)
7474
{
7575
zend_object *timezone_object = nullptr;
7676
zend_string *timezone_string = nullptr;
7777
char *locale_str = NULL;
7878
size_t locale_len = 0;
7979
UErrorCode status = U_ZERO_ERROR;
80-
intl_error_reset(NULL);
8180

8281
ZEND_PARSE_PARAMETERS_START(0, 2)
8382
Z_PARAM_OPTIONAL
@@ -158,15 +157,14 @@ class BugStringCharEnumeration : public StringEnumeration
158157
};
159158
UOBJECT_DEFINE_RTTI_IMPLEMENTATION(BugStringCharEnumeration)
160159

161-
U_CFUNC PHP_FUNCTION(intlcal_get_keyword_values_for_locale)
160+
PHP_INTL_FUNCTION_WITH_ERROR_RESET(intlcal_get_keyword_values_for_locale)
162161
{
163162
UErrorCode status = U_ZERO_ERROR;
164163
char *key,
165164
*locale;
166165
size_t key_len,
167166
locale_len;
168167
bool commonly_used;
169-
intl_error_reset(NULL);
170168

171169
ZEND_PARSE_PARAMETERS_START(3, 3)
172170
Z_PARAM_STRING(key, key_len)
@@ -186,19 +184,15 @@ U_CFUNC PHP_FUNCTION(intlcal_get_keyword_values_for_locale)
186184
IntlIterator_from_StringEnumeration(se, return_value);
187185
}
188186

189-
U_CFUNC PHP_FUNCTION(intlcal_get_now)
187+
PHP_INTL_FUNCTION_WITH_ERROR_RESET(intlcal_get_now)
190188
{
191-
intl_error_reset(NULL);
192-
193189
ZEND_PARSE_PARAMETERS_NONE();
194190

195191
RETURN_DOUBLE((double)Calendar::getNow());
196192
}
197193

198-
U_CFUNC PHP_FUNCTION(intlcal_get_available_locales)
194+
PHP_INTL_FUNCTION_WITH_ERROR_RESET(intlcal_get_available_locales)
199195
{
200-
intl_error_reset(NULL);
201-
202196
ZEND_PARSE_PARAMETERS_NONE();
203197

204198
int32_t count;
@@ -1018,7 +1012,7 @@ U_CFUNC PHP_FUNCTION(intlcal_set_skipped_wall_time_option)
10181012
RETURN_TRUE;
10191013
}
10201014

1021-
U_CFUNC PHP_FUNCTION(intlcal_from_date_time)
1015+
PHP_INTL_FUNCTION_WITH_ERROR_RESET(intlcal_from_date_time)
10221016
{
10231017
zend_object *date_obj;
10241018
zend_string *date_str;
@@ -1029,7 +1023,6 @@ U_CFUNC PHP_FUNCTION(intlcal_from_date_time)
10291023
TimeZone *timeZone;
10301024
UErrorCode status = U_ZERO_ERROR;
10311025
Calendar *cal;
1032-
intl_error_reset(NULL);
10331026

10341027
ZEND_PARSE_PARAMETERS_START(1, 2)
10351028
Z_PARAM_OBJ_OF_CLASS_OR_STR(date_obj, php_date_get_date_ce(), date_str)

ext/intl/locale/locale_methods.cpp

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -836,7 +836,7 @@ U_CFUNC PHP_FUNCTION(locale_get_display_keyword_value)
836836
/* {{{ return an associative array containing keyword-value
837837
* pairs for this locale. The keys are keys to the array (doh!)
838838
*/
839-
U_CFUNC PHP_FUNCTION( locale_get_keywords )
839+
PHP_INTL_FUNCTION_WITH_ERROR_RESET(locale_get_keywords)
840840
{
841841
UEnumeration* e = NULL;
842842
UErrorCode status = U_ZERO_ERROR;
@@ -847,7 +847,6 @@ U_CFUNC PHP_FUNCTION( locale_get_keywords )
847847
char* loc_name = NULL;
848848
size_t loc_name_len = 0;
849849

850-
intl_error_reset( NULL );
851850

852851
ZEND_PARSE_PARAMETERS_START(1, 1)
853852
Z_PARAM_PATH(loc_name, loc_name_len)
@@ -1051,15 +1050,14 @@ static int handleAppendResult( int result, smart_str* loc_name)
10511050
* }}} */
10521051
/* {{{ Creates a locale by combining the parts of locale-ID passed
10531052
* }}} */
1054-
U_CFUNC PHP_FUNCTION(locale_compose)
1053+
PHP_INTL_FUNCTION_WITH_ERROR_RESET(locale_compose)
10551054
{
10561055
smart_str loc_name_s = {NULL, 0};
10571056
smart_str *loc_name = &loc_name_s;
10581057
zval* arr = NULL;
10591058
HashTable* hash_arr = NULL;
10601059
int result = 0;
10611060

1062-
intl_error_reset( NULL );
10631061

10641062
ZEND_PARSE_PARAMETERS_START(1, 1)
10651063
Z_PARAM_ARRAY(arr)
@@ -1232,13 +1230,12 @@ static int add_array_entry(const char* loc_name, zval* hash_arr, const char* key
12321230
/* }}} */
12331231

12341232
/* {{{ parses a locale-id into an array the different parts of it */
1235-
U_CFUNC PHP_FUNCTION(locale_parse)
1233+
PHP_INTL_FUNCTION_WITH_ERROR_RESET(locale_parse)
12361234
{
12371235
char* loc_name = NULL;
12381236
size_t loc_name_len = 0;
12391237
int grOffset = 0;
12401238

1241-
intl_error_reset( NULL );
12421239

12431240
ZEND_PARSE_PARAMETERS_START(1, 1)
12441241
Z_PARAM_PATH(loc_name, loc_name_len)
@@ -1268,7 +1265,7 @@ U_CFUNC PHP_FUNCTION(locale_parse)
12681265
/* }}} */
12691266

12701267
/* {{{ gets an array containing the list of variants, or null */
1271-
U_CFUNC PHP_FUNCTION(locale_get_all_variants)
1268+
PHP_INTL_FUNCTION_WITH_ERROR_RESET(locale_get_all_variants)
12721269
{
12731270
char* loc_name = NULL;
12741271
size_t loc_name_len = 0;
@@ -1278,7 +1275,6 @@ U_CFUNC PHP_FUNCTION(locale_get_all_variants)
12781275
zend_string* variant = NULL;
12791276
char* saved_ptr = NULL;
12801277

1281-
intl_error_reset( NULL );
12821278

12831279
ZEND_PARSE_PARAMETERS_START(1, 1)
12841280
Z_PARAM_PATH(loc_name, loc_name_len)
@@ -1352,7 +1348,7 @@ static int strToMatch(const char* str ,char *retstr)
13521348
/* {{{ Checks if a $langtag filter matches with $locale according to RFC 4647's basic filtering algorithm */
13531349
/* }}} */
13541350
/* {{{ Checks if a $langtag filter matches with $locale according to RFC 4647's basic filtering algorithm */
1355-
U_CFUNC PHP_FUNCTION(locale_filter_matches)
1351+
PHP_INTL_FUNCTION_WITH_ERROR_RESET(locale_filter_matches)
13561352
{
13571353
char* lang_tag = NULL;
13581354
size_t lang_tag_len = 0;
@@ -1372,7 +1368,6 @@ U_CFUNC PHP_FUNCTION(locale_filter_matches)
13721368
bool boolCanonical = 0;
13731369
UErrorCode status = U_ZERO_ERROR;
13741370

1375-
intl_error_reset( NULL );
13761371

13771372
ZEND_PARSE_PARAMETERS_START(2, 3)
13781373
Z_PARAM_PATH(lang_tag, lang_tag_len)
@@ -1640,7 +1635,7 @@ static zend_string* lookup_loc_range(const char* loc_range, HashTable* hash_arr,
16401635
/* {{{ Searches the items in $langtag for the best match to the language
16411636
* range
16421637
*/
1643-
U_CFUNC PHP_FUNCTION(locale_lookup)
1638+
PHP_INTL_FUNCTION_WITH_ERROR_RESET(locale_lookup)
16441639
{
16451640
zend_string* fallback_loc_str = NULL;
16461641
char* loc_range = NULL;
@@ -1651,7 +1646,6 @@ U_CFUNC PHP_FUNCTION(locale_lookup)
16511646
bool boolCanonical = 0;
16521647
zend_string* result_str = NULL;
16531648

1654-
intl_error_reset( NULL );
16551649

16561650
ZEND_PARSE_PARAMETERS_START(2, 4)
16571651
Z_PARAM_ARRAY(arr)

ext/intl/normalizer/normalizer_normalize.cpp

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ static UBool intl_is_normalized(zend_long form, const UChar *uinput, int32_t uin
7373
}/*}}}*/
7474

7575
/* {{{ Normalize a string. */
76-
U_CFUNC PHP_FUNCTION( normalizer_normalize )
76+
PHP_INTL_FUNCTION_WITH_ERROR_RESET(normalizer_normalize)
7777
{
7878
char* input = NULL;
7979
/* form is optional, defaults to FORM_C */
@@ -92,7 +92,6 @@ U_CFUNC PHP_FUNCTION( normalizer_normalize )
9292

9393
int32_t size_needed;
9494

95-
intl_error_reset( NULL );
9695

9796
/* Parse parameters. */
9897
if( zend_parse_method_parameters( ZEND_NUM_ARGS(), getThis(), "s|l",
@@ -202,7 +201,7 @@ U_CFUNC PHP_FUNCTION( normalizer_normalize )
202201
/* }}} */
203202

204203
/* {{{ Test if a string is in a given normalization form. */
205-
U_CFUNC PHP_FUNCTION( normalizer_is_normalized )
204+
PHP_INTL_FUNCTION_WITH_ERROR_RESET(normalizer_is_normalized)
206205
{
207206
char* input = NULL;
208207
/* form is optional, defaults to FORM_C */
@@ -215,7 +214,6 @@ U_CFUNC PHP_FUNCTION( normalizer_is_normalized )
215214

216215
UBool uret = false;
217216

218-
intl_error_reset( NULL );
219217

220218
/* Parse parameters. */
221219
if( zend_parse_method_parameters( ZEND_NUM_ARGS(), getThis(), "s|l",
@@ -278,7 +276,7 @@ U_CFUNC PHP_FUNCTION( normalizer_is_normalized )
278276
/* }}} */
279277

280278
/* {{{ Returns the Decomposition_Mapping property for the given UTF-8 encoded code point. */
281-
U_CFUNC PHP_FUNCTION( normalizer_get_raw_decomposition )
279+
PHP_INTL_FUNCTION_WITH_ERROR_RESET(normalizer_get_raw_decomposition)
282280
{
283281
char* input = NULL;
284282
size_t input_length = 0;
@@ -293,7 +291,6 @@ U_CFUNC PHP_FUNCTION( normalizer_get_raw_decomposition )
293291

294292
zend_long form = NORMALIZER_DEFAULT;
295293

296-
intl_error_reset(NULL);
297294

298295
ZEND_PARSE_PARAMETERS_START(1, 2)
299296
Z_PARAM_STRING(input, input_length)

ext/intl/php_intl.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,15 @@ PHP_MINFO_FUNCTION(intl);
7171
const char *intl_locale_get_default( void );
7272
char *canonicalize_locale_string(const char* locale);
7373

74+
#define PHP_INTL_FUNCTION_WITH_ERROR_RESET(name) \
75+
static void php_intl_##name##_impl(INTERNAL_FUNCTION_PARAMETERS); \
76+
U_CFUNC PHP_FUNCTION(name) \
77+
{ \
78+
intl_error_reset(NULL); \
79+
php_intl_##name##_impl(INTERNAL_FUNCTION_PARAM_PASSTHRU); \
80+
} \
81+
static void php_intl_##name##_impl(INTERNAL_FUNCTION_PARAMETERS)
82+
7483
#define PHP_INTL_VERSION PHP_VERSION
7584

7685
#endif /* PHP_INTL_H */

ext/intl/resourcebundle/resourcebundle_class.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,7 @@ U_CFUNC PHP_FUNCTION( resourcebundle_count )
333333
}
334334

335335
/* {{{ Get available locales from ResourceBundle name */
336-
U_CFUNC PHP_FUNCTION( resourcebundle_locales )
336+
PHP_INTL_FUNCTION_WITH_ERROR_RESET(resourcebundle_locales)
337337
{
338338
char * bundlename;
339339
size_t bundlename_len = 0;
@@ -343,8 +343,6 @@ U_CFUNC PHP_FUNCTION( resourcebundle_locales )
343343
UEnumeration *icuenum;
344344
UErrorCode icuerror = U_ZERO_ERROR;
345345

346-
intl_errors_reset( NULL );
347-
348346
ZEND_PARSE_PARAMETERS_START(1, 1)
349347
Z_PARAM_STRING(bundlename, bundlename_len)
350348
ZEND_PARSE_PARAMETERS_END();

0 commit comments

Comments
 (0)