On Mon, 21 Jul 2025 15:38:36 GMT, Matthias Baesken <mbaes...@openjdk.org> wrote:
>> test/jdk/java/lang/String/nativeEncoding/libstringPlatformChars.c line 74:
>>
>>> 72: (*env)->ReleasePrimitiveArrayCritical(env, bytes, (void*)jbytes, 0);
>>> 73:
>>> 74: jstring res = JNU_NewStringPlatform(env, str);
>>
>> At line 66: where it returns null, it seem like it should also be freeing
>> `str`. (The analyzer didn't catch that?)
>
> It catches that too
>
> /jdk/test/jdk/java/lang/String/nativeEncoding/libstringPlatformChars.c:65:8:
> warning: leak of 'str' [CWE-401] [-Wanalyzer-malloc-leak]
> 65 | if (jbytes == NULL) {
>
>
> and also
>
>
> /jdk/test/jdk/java/lang/String/nativeEncoding/libstringPlatformChars.c:69:16:
> warning: dereference of possibly-NULL 'str' [CWE-690]
> [-Wanalyzer-possible-null-dereference]
> 69 | str[i] = (char)jbytes[i];
> | ~~~~~~~^~~~~~~~~~~~~~~~~
>
>
> I just overlooked it because a log of a full build with '-fanalyzer'
> enabled contains quite a lot of output (the event explanations of every
> finding are rather long).
Seems better to defer the `malloc` until after you know `jbytes` is not null;
and also check the `malloc` result for null.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/26415#discussion_r2221292734
