ICU-20832 use uint32_t instead of uint16_t to avoid overflows for very long strings #840
Conversation
|
|
| @@ -317,7 +317,7 @@ inline uint16_t initializePatternCETable(UStringSearch *strsrch, | |||
| uprv_free(pattern->ces); | |||
| } | |||
|
|
|||
| uint16_t offset = 0; | |||
| uint32_t offset = 0; | |||
There was a problem hiding this comment.
Hmm. There is inconsistency in the places this offset value is used; some are int32_t (like pattern->cesLength, pattern->pcesLength) and some are uint32_t (like the corresponding parameter in addTouint64_tArray). We may need to be more consistent about usage. But I guess that could be later fix, the chanage proposed here does solve the immediate overflow problem
There was a problem hiding this comment.
Thanks Peter. I could file a follow up ticket for 66 or 67, and this ticket could be just for the maint-65 release.
There was a problem hiding this comment.
I filed a follow-up ticket: https://unicode-org.atlassian.net/browse/ICU-20833
Aside: Since addTouint64_tArray uses uint32_t I wonder if we'd want to use that everywhere in here?
There was a problem hiding this comment.
Yeah, probably uint32_t everywhere is best. Thanks for the followup! Will approve this one for 65.
| @@ -388,7 +388,7 @@ inline uint16_t initializePatternPCETable(UStringSearch *strsrch, | |||
| uprv_free(pattern->pces); | |||
| } | |||
|
|
|||
| uint16_t offset = 0; | |||
| uint32_t offset = 0; | |||
There was a problem hiding this comment.
Same comment as above about int32_t vs uint32_t.
|
Thank you for reporting the issue (filing a ticket/issue) and for creating the PR! In the ICU-TC call this ticket/PR was discussed and the general consensus was that this would be okay for the I'll change the target branch on this PR to be into the maint branch in moment. |
Checklist