-
-
Notifications
You must be signed in to change notification settings - Fork 736
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.
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
ICU-20065 Prevent crash on Collator::makeInstance fail when optimized… #54
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks Peter!
icu4c/source/i18n/coll.cpp
Outdated
@@ -448,6 +448,13 @@ Collator* U_EXPORT2 Collator::createInstance(const Locale& desiredLocale, | |||
#endif | |||
{ | |||
coll = makeInstance(desiredLocale, status); | |||
// Either returns NULL with U_FAILURE(status), or non-NULL with U_SUCCESS(status) | |||
} | |||
// The use of *coll in setAttributesFromKeywords can cause causes the NULL check |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor nit: "can cause causes" -- the wording feels a bit odd.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes, I changed how I wanted to say this midstream and forgot to delete the old wording. Fixed with another commit.
ICU-20065 Prevent crash on Collator::makeInstance fail when optimized…
ICU-20065 Prevent crash on Collator::makeInstance fail when optimized…
ICU-20065 Prevent crash on Collator::makeInstance fail when optimized…
ICU-20065 Prevent crash on Collator::makeInstance fail when optimized…
ICU-20065 Prevent crash on Collator::makeInstance fail when optimized…
ICU-20065 Prevent crash on Collator::makeInstance fail when optimized…
Fix compiler warnings
In Collator::createInstance, before the call to setAttributesFromKeywords, Collator* Coll will be NULL if U_FAILURE(status). In that case (FAILURE status), setAttributesFromKeywords returns immediately and does not use the dereferenced value *coll. However, the fact that Coll is dereferenced allows C++ compilers to optimize out the NULL check in the subsequent delete. In that case a failure in makeInstance due to a bad key-value such as "collation=private-kana" will cause the "delete Coll" to crash. So we add a seemingly-redundant check for U_FAILURE(status), and return NULL if TRUE, before the call to setAttributesFromKeywords.
Checklist