ICU-21383 Fix memory problem in FormattedStringBuilder

[sffc]

The fix to the reported issue, a list with more than 8 elements, is the addition of an argument to the .resize() call in appendSpanInfo. The other fix, at the bottom of nextPositionImpl, came up in Valgrind during testing and is triggered when calling .nextPosition() repeatedly after it has returned false.

Checklist

• Issue filed: https://unicode-org.atlassian.net/browse/ICU-21383
• Updated PR title and link in previous line to include Issue number
• Issue accepted
• Tests included
• Documentation is changed or added

[sffc]

FYI @hugovdm: a bug one can make when using MaybeStackArray: don't forget to pass the optional length parameter to the resize function!

[hugovdm]

Looks good to me, except that the C++ and Java unit tests have falling out of sync?

[jira-pull-request-webhook]

Notice: the branch changed across the force-push!

• icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/ListFormatterTest.java is now changed in the branch

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

[sffc]

OK, I added the corresponding Java tests via amend. This was an ICU4C-specific bug, but it doesn't hurt to add the ICU4J tests, since it helps to keep the two unit test suites in sync, as you pointed out.

[FrankYFTang]

Please change the U_ASSERT condition to >

[hugovdm]

Looks good to me. (The assert also made sense to me.)

The MSYS2 failure looks unrelated to this PR:

configure: error: in `/d/a/1/s/icu4c/source':
configure: error: no acceptable C compiler found in $PATH

[jefgen]

The MSYS2 failure looks unrelated to this PR:

configure: error: in `/d/a/1/s/icu4c/source':
configure: error: no acceptable C compiler found in $PATH

Hmm.. I'll try to re-run the build again...

[markusicu]

lgtm

[sffc]

Thanks for all the reviews. However, I caught two mistakes which I fixed in another amendment:

1. I need to change this behavior in prependSpanInfo too, not just appendSpanInfo
2. .resize() returns nullptr if it fails to allocate memory, but I'm not handling that case

Please re-review.

[jira-pull-request-webhook]

Notice: the branch changed across the force-push!

• icu4c/source/i18n/formattedval_impl.h is now changed in the branch
• icu4c/source/i18n/formattedval_sbimpl.cpp is different
• icu4c/source/i18n/listformatter.cpp is now changed in the branch

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

[sffc]

Note: The Jira-GitHub bot seems to have been down for a time. I manually re-sent the webhook request, which is why it was delayed.