ICU-22377 Fix errors found by gcc -fanalyzer

[nvinson]

Found using gcc -fanalyze.

Code built using the following configure command:

../configure --prefix=/usr --build=x86_64-pc-linux-gnu --host=x86_64-pc-linux-gnu --mandir=/usr/share/man --infodir=/usr/share/info --datadir=/usr/share --sysconfdir=/etc --localstatedir=/var/lib --datarootdir=/usr/share --docdir=/usr/share/doc/icu-73.1-r1 --htmldir=/usr/share/doc/icu-73.1-r1/html --libdir=/usr/lib64 --disable-renaming --disable-layoutex --disable-debug --disable-static --disable-tests --disable-samples build_alias=x86_64-pc-linux-gnu host_alias=x86_64-pc-linux-gnu CC=x86_64-pc-linux-gnu-gcc CFLAGS="-march=native -O2 -pipe -fanalyzer -Werror=analyzer-va-arg-type-mismatch -Werror=analyzer-va-list-exhausted -Werror=analyzer-va-list-leak -Werror=analyzer-va-list-use-after-va-end" LDFLAGS="-Wl,-O1 -Wl,--as-needed" CXX=x86_64-pc-linux-gnu-g++ CXXFLAGS="-march=native -O2 -pipe -Werror=analyzer-va-arg-type-mismatch -Werror=analyzer-va-list-exhausted -Werror=analyzer-va-list-leak -Werror=analyzer-va-list-use-after-va-end -std=c++14"

Checklist

• Required: Issue filed: https://unicode-org.atlassian.net/browse/ICU-22377
• Required: The PR title must be prefixed with a JIRA Issue number.
• Required: The PR description must include the link to the Jira Issue, for example by completing the URL in the first checklist item
• Required: Each commit message must be prefixed with a JIRA Issue number.
• Issue accepted (done by Technical Committee after discussion)
• Tests included, if applicable
• API docs and/or User Guide docs changed or added, if applicable

[jira-pull-request-webhook]

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot

[jira-pull-request-webhook]

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot

[roubert]

I suggest that you put both of the missing va_end() calls directly before their corresponding return statements, for that'll make the code a little bit easier to follow.

Then I also suggest that you don't touch the strncpy() calls at all, for while the -Wstringop-truncation warning certainly is able to find problematic code this is only a warning (it won't cause any build failure, not even with GCC 13 and your set of compiler flags) and there isn't actually anything wrong with this code here and the same warning shows up in other source files as well, so it doesn't make much sense to change just these calls in this particular file now.

See the commit that I just added to this PR, you can squash that together with your commit.

[jira-pull-request-webhook]

Notice: the branch changed across the force-push!

• icu4c/source/tools/ctestfw/ctest.c is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

[nvinson]

I suggest that you put both of the missing va_end() calls directly before their corresponding return statements, for that'll make the code a little bit easier to follow.

I reviewed the commit, and while I was reviewing I was reminded that the C standard says "Each invocation of the va_start and va_copy macros shall be matched by a corresponding invocation of the va_end macro in the same function." Therefore, I redid my commit keeping that rule in mind. This makes for a slightly larger patch as some of the existing va_end() macros had to be moved to comply with the standard.

Then I also suggest that you don't touch the strncpy() calls at all, for while the -Wstringop-truncation warning certainly is able to find problematic code this is only a warning (it won't cause any build failure, not even with GCC 13 and your set of compiler flags) and there isn't actually anything wrong with this code here and the same warning shows up in other source files as well, so it doesn't make much sense to change just these calls in this particular file now.

I have reverted the strncpy() call changes. That said, I ran gcc -fanalzyer over the 73.1 release code and derived my patches from those results and fixed everything I saw in the analyzer output. Assuming I didn't miss anything, the fact that there are additional such warnings means that those additional problematic calls were added post 73.1 release. I personally find that concerning and would strongly recommend a review of all strncpy() calls.

See the commit that I just added to this PR, you can squash that together with your commit.

[jira-pull-request-webhook]

Notice: the branch changed across the force-push!

• icu4c/source/tools/ctestfw/ctest.c is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

[roubert]

LGTM