Skip to content

intl extension couple of micro optimisations for error edge cases. #10044

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 14, 2023
Merged

intl extension couple of micro optimisations for error edge cases. #10044

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions ext/intl/breakiterator/breakiterator_methods.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ static void _breakiter_int32_ret_int32(

BREAKITER_METHOD_FETCH_OBJECT;

if (arg < INT32_MIN || arg > INT32_MAX) {
if (UNEXPECTED(arg < INT32_MIN || arg > INT32_MAX)) {
zend_argument_value_error(1, "must be between %d and %d", INT32_MIN, INT32_MAX);
RETURN_THROWS();
}
Expand Down Expand Up @@ -292,7 +292,7 @@ U_CFUNC PHP_METHOD(IntlBreakIterator, isBoundary)
RETURN_THROWS();
}

if (offset < INT32_MIN || offset > INT32_MAX) {
if (UNEXPECTED(offset < INT32_MIN || offset > INT32_MAX)) {
zend_argument_value_error(1, "must be between %d and %d", INT32_MIN, INT32_MAX);
RETURN_THROWS();
}
Expand Down
2 changes: 1 addition & 1 deletion ext/intl/calendar/calendar_class.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ static zend_object *Calendar_clone_obj(zend_object *object)
Calendar *newCalendar;

newCalendar = co_orig->ucal->clone();
if (!newCalendar) {
if (UNEXPECTED(!newCalendar)) {
zend_string *err_msg;
intl_errors_set_code(CALENDAR_ERROR_P(co_orig),
U_MEMORY_ALLOCATION_ERROR);
Expand Down
10 changes: 5 additions & 5 deletions ext/intl/calendar/calendar_methods.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ using icu::Locale;
}

#define ZEND_VALUE_ERROR_OUT_OF_BOUND_VALUE(argument, zpp_arg_position) \
if (argument < INT32_MIN || argument > INT32_MAX) { \
if (UNEXPECTED(argument < INT32_MIN || argument > INT32_MAX)) { \
zend_argument_value_error(getThis() ? ((zpp_arg_position)-1) : zpp_arg_position, \
"must be between %d and %d", INT32_MIN, INT32_MAX); \
RETURN_THROWS(); \
Expand Down Expand Up @@ -96,7 +96,7 @@ U_CFUNC PHP_FUNCTION(intlcal_create_instance)

Calendar *cal = Calendar::createInstance(timeZone,
Locale::createFromName(locale_str), status);
if (cal == NULL) {
if (UNEXPECTED(cal == NULL)) {
delete timeZone;
intl_error_set(NULL, status, "Error creating ICU Calendar object", 0);
RETURN_NULL();
Expand Down Expand Up @@ -637,7 +637,7 @@ U_CFUNC PHP_FUNCTION(intlcal_get_time_zone)
CALENDAR_METHOD_FETCH_OBJECT;

TimeZone *tz = co->ucal->getTimeZone().clone();
if (tz == NULL) {
if (UNEXPECTED(tz == NULL)) {
intl_errors_set(CALENDAR_ERROR_P(co), U_MEMORY_ALLOCATION_ERROR,
"intlcal_get_time_zone: could not clone TimeZone", 0);
RETURN_FALSE;
Expand Down Expand Up @@ -1000,7 +1000,7 @@ U_CFUNC PHP_FUNCTION(intlcal_from_date_time)

cal = Calendar::createInstance(timeZone,
Locale::createFromName(locale_str), status);
if (cal == NULL) {
if (UNEXPECTED(cal == NULL)) {
delete timeZone;
intl_error_set(NULL, status, "intlcal_from_date_time: "
"error creating ICU Calendar object", 0);
Expand Down Expand Up @@ -1045,7 +1045,7 @@ U_CFUNC PHP_FUNCTION(intlcal_to_date_time)

INTL_METHOD_CHECK_STATUS(co, "Call to ICU method has failed");

if (date > (double)U_INT64_MAX || date < (double)U_INT64_MIN) {
if (UNEXPECTED(date > (double)U_INT64_MAX || date < (double)U_INT64_MIN)) {
intl_errors_set(CALENDAR_ERROR_P(co), U_ILLEGAL_ARGUMENT_ERROR,
"intlcal_to_date_time: The calendar date is out of the "
"range for a 64-bit integer", 0);
Expand Down
4 changes: 2 additions & 2 deletions ext/intl/calendar/gregoriancalendar_methods.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ static void _php_intlgregcal_constructor_body(
} else {
// From date/time (3, 5 or 6 arguments)
for (int i = 0; i < variant; i++) {
if (largs[i] < INT32_MIN || largs[i] > INT32_MAX) {
if (UNEXPECTED(largs[i] < INT32_MIN || largs[i] > INT32_MAX)) {
zend_argument_value_error(getThis() ? (i-1) : i,
"must be between %d and %d", INT32_MIN, INT32_MAX);
RETURN_THROWS();
Expand Down Expand Up @@ -251,7 +251,7 @@ U_CFUNC PHP_FUNCTION(intlgregcal_is_leap_year)
RETURN_THROWS();
}

if (year < INT32_MIN || year > INT32_MAX) {
if (UNEXPECTED(year < INT32_MIN || year > INT32_MAX)) {
zend_argument_value_error(getThis() ? 1 : 2, "must be between %d and %d", INT32_MIN, INT32_MAX);
RETURN_THROWS();
}
Expand Down
8 changes: 4 additions & 4 deletions ext/intl/dateformat/dateformat_attrcpp.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ U_CFUNC PHP_FUNCTION(datefmt_get_timezone)

const TimeZone& tz = fetch_datefmt(dfo)->getTimeZone();
TimeZone *tz_clone = tz.clone();
if (tz_clone == NULL) {
if (UNEXPECTED(tz_clone == NULL)) {
intl_errors_set(INTL_DATA_ERROR_P(dfo), U_MEMORY_ALLOCATION_ERROR,
"datefmt_get_timezone: Out of memory when cloning time zone",
0);
Expand Down Expand Up @@ -142,7 +142,7 @@ U_CFUNC PHP_FUNCTION(datefmt_get_calendar_object)
}

Calendar *cal_clone = cal->clone();
if (cal_clone == NULL) {
if (UNEXPECTED(cal_clone == NULL)) {
intl_errors_set(INTL_DATA_ERROR_P(dfo), U_MEMORY_ALLOCATION_ERROR,
"datefmt_get_calendar_object: Out of memory when cloning "
"calendar", 0);
Expand Down Expand Up @@ -193,7 +193,7 @@ U_CFUNC PHP_FUNCTION(datefmt_set_calendar)
if (cal_owned) {
/* a non IntlCalendar was specified, we want to keep the timezone */
TimeZone *old_timezone = fetch_datefmt(dfo)->getTimeZone().clone();
if (old_timezone == NULL) {
if (UNEXPECTED(old_timezone == NULL)) {
intl_errors_set(INTL_DATA_ERROR_P(dfo), U_MEMORY_ALLOCATION_ERROR,
"datefmt_set_calendar: Out of memory when cloning calendar",
0);
Expand All @@ -203,7 +203,7 @@ U_CFUNC PHP_FUNCTION(datefmt_set_calendar)
cal->adoptTimeZone(old_timezone);
} else {
cal = cal->clone();
if (cal == NULL) {
if (UNEXPECTED(cal == NULL)) {
intl_errors_set(INTL_DATA_ERROR_P(dfo), U_MEMORY_ALLOCATION_ERROR,
"datefmt_set_calendar: Out of memory when cloning calendar",
0);
Expand Down
2 changes: 1 addition & 1 deletion ext/intl/dateformat/dateformat_format_object.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ using icu::GregorianCalendar;
using icu::StringPiece;
using icu::SimpleDateFormat;

static const DateFormat::EStyle valid_styles[] = {
static constexpr DateFormat::EStyle valid_styles[] = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

constexpr is a C++11 feature; I'm not sure whether we can require it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't the issue that REHL 7 is still not EOL and the GCC version which comes bundled doesn't support C11/C++11 fully?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

g++ 4.8 supports the vast majority of C++ 11 features, we already use smart pointers, another C++11 nicety, since 8.2.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, well then I suppose it should be fine?

DateFormat::kNone,
DateFormat::kFull,
DateFormat::kLong,
Expand Down
4 changes: 2 additions & 2 deletions ext/intl/intl_convertcpp.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ extern "C" {
/* {{{ intl_stringFromChar */
int intl_stringFromChar(UnicodeString &ret, char *str, size_t str_len, UErrorCode *status)
{
if(str_len > INT32_MAX) {
if(UNEXPECTED(str_len > INT32_MAX)) {
*status = U_BUFFER_OVERFLOW_ERROR;
ret.setToBogus();
return FAILURE;
Expand Down Expand Up @@ -56,7 +56,7 @@ zend_string* intl_charFromString(const UnicodeString &from, UErrorCode *status)
{
zend_string *u8res;

if (from.isBogus()) {
if (UNEXPECTED(from.isBogus())) {
return NULL;
}

Expand Down
16 changes: 8 additions & 8 deletions ext/intl/msgformat/msgformat_helpers.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ static void umsg_set_timezone(MessageFormatter_object *mfo,

formats = mf->getFormats(count);

if (formats == NULL) {
if (UNEXPECTED(formats == NULL)) {
intl_errors_set(&err, U_MEMORY_ALLOCATION_ERROR,
"Out of memory retrieving subformats", 0);
}
Expand Down Expand Up @@ -403,7 +403,7 @@ U_CFUNC void umsg_format_helper(MessageFormatter_object *mfo,
/* Process key and retrieve type */
if (str_index == NULL) {
/* includes case where index < 0 because it's exposed as unsigned */
if (num_index > (zend_ulong)INT32_MAX) {
if (UNEXPECTED(num_index > (zend_ulong)INT32_MAX)) {
intl_errors_set(&err, U_ILLEGAL_ARGUMENT_ERROR,
"Found negative or too large array key", 0);
continue;
Expand Down Expand Up @@ -477,17 +477,17 @@ U_CFUNC void umsg_format_helper(MessageFormatter_object *mfo,
int32_t tInt32 = 0;

if (Z_TYPE_P(elem) == IS_DOUBLE) {
if (Z_DVAL_P(elem) > (double)INT32_MAX ||
Z_DVAL_P(elem) < (double)INT32_MIN) {
if (UNEXPECTED(Z_DVAL_P(elem) > (double)INT32_MAX ||
Z_DVAL_P(elem) < (double)INT32_MIN)) {
intl_errors_set(&err, U_ILLEGAL_ARGUMENT_ERROR,
"Found PHP float with absolute value too large for "
"32 bit integer argument", 0);
} else {
tInt32 = (int32_t)Z_DVAL_P(elem);
}
} else if (Z_TYPE_P(elem) == IS_LONG) {
if (Z_LVAL_P(elem) > INT32_MAX ||
Z_LVAL_P(elem) < INT32_MIN) {
if (UNEXPECTED(Z_LVAL_P(elem) > INT32_MAX ||
Z_LVAL_P(elem) < INT32_MIN)) {
intl_errors_set(&err, U_ILLEGAL_ARGUMENT_ERROR,
"Found PHP integer with absolute value too large "
"for 32 bit integer argument", 0);
Expand All @@ -505,8 +505,8 @@ U_CFUNC void umsg_format_helper(MessageFormatter_object *mfo,
int64_t tInt64 = 0;

if (Z_TYPE_P(elem) == IS_DOUBLE) {
if (Z_DVAL_P(elem) > (double)U_INT64_MAX ||
Z_DVAL_P(elem) < (double)U_INT64_MIN) {
if (UNEXPECTED(Z_DVAL_P(elem) > (double)U_INT64_MAX ||
Z_DVAL_P(elem) < (double)U_INT64_MIN)) {
intl_errors_set(&err, U_ILLEGAL_ARGUMENT_ERROR,
"Found PHP float with absolute value too large for "
"64 bit integer argument", 0);
Expand Down
4 changes: 2 additions & 2 deletions ext/intl/timezone/timezone_class.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ U_CFUNC TimeZone *timezone_process_timezone_argument(zval *zv_timezone,
return NULL;
}
timeZone = to->utimezone->clone();
if (timeZone == NULL) {
if (UNEXPECTED(timeZone == NULL)) {
spprintf(&message, 0, "%s: could not clone TimeZone", func);
if (message) {
intl_errors_set(outside_error, U_MEMORY_ALLOCATION_ERROR, message, 1);
Expand Down Expand Up @@ -193,7 +193,7 @@ U_CFUNC TimeZone *timezone_process_timezone_argument(zval *zv_timezone,
return NULL;
}
timeZone = TimeZone::createTimeZone(id);
if (timeZone == NULL) {
if (UNEXPECTED(timeZone == NULL)) {
spprintf(&message, 0, "%s: Could not create time zone", func);
if (message) {
intl_errors_set(outside_error, U_MEMORY_ALLOCATION_ERROR, message, 1);
Expand Down
10 changes: 5 additions & 5 deletions ext/intl/timezone/timezone_methods.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,8 @@ U_CFUNC PHP_FUNCTION(intltz_create_enumeration)
se = TimeZone::createEnumeration();
} else if (Z_TYPE_P(arg) == IS_LONG) {
int_offset:
if (Z_LVAL_P(arg) < (zend_long)INT32_MIN ||
Z_LVAL_P(arg) > (zend_long)INT32_MAX) {
if (UNEXPECTED(Z_LVAL_P(arg) < (zend_long)INT32_MIN ||
Z_LVAL_P(arg) > (zend_long)INT32_MAX)) {
intl_error_set(NULL, U_ILLEGAL_ARGUMENT_ERROR,
"intltz_create_enumeration: value is out of range", 0);
RETURN_FALSE;
Expand Down Expand Up @@ -241,7 +241,7 @@ U_CFUNC PHP_FUNCTION(intltz_create_time_zone_id_enumeration)
}

if (!arg3isnull) {
if (offset_arg < (zend_long)INT32_MIN || offset_arg > (zend_long)INT32_MAX) {
if (UNEXPECTED(offset_arg < (zend_long)INT32_MIN || offset_arg > (zend_long)INT32_MAX)) {
intl_error_set(NULL, U_ILLEGAL_ARGUMENT_ERROR,
"intltz_create_time_zone_id_enumeration: offset out of bounds", 0);
RETURN_FALSE;
Expand Down Expand Up @@ -350,7 +350,7 @@ U_CFUNC PHP_FUNCTION(intltz_get_equivalent_id)
RETURN_THROWS();
}

if (index < (zend_long)INT32_MIN || index > (zend_long)INT32_MAX) {
if (UNEXPECTED(index < (zend_long)INT32_MIN || index > (zend_long)INT32_MAX)) {
RETURN_FALSE;
}

Expand Down Expand Up @@ -475,7 +475,7 @@ U_CFUNC PHP_FUNCTION(intltz_has_same_rules)
RETURN_BOOL(to->utimezone->hasSameRules(*other_to->utimezone));
}

static const TimeZone::EDisplayType display_types[] = {
static constexpr TimeZone::EDisplayType display_types[] = {
TimeZone::SHORT, TimeZone::LONG,
TimeZone::SHORT_GENERIC, TimeZone::LONG_GENERIC,
TimeZone::SHORT_GMT, TimeZone::LONG_GMT,
Expand Down