RFR: 8339783: Implement JEP 479: Remove the Windows 32-bit x86 Port [v30]
Kim Barrett
kbarrett at openjdk.org
Wed Nov 6 21:28:00 UTC 2024
On Wed, 6 Nov 2024 15:27:16 GMT, Magnus Ihse Bursie <ihse at openjdk.org> wrote:
>> src/java.base/share/native/libjava/NativeLibraries.c line 67:
>>
>>> 65: strcat(jniEntryName, "_");
>>> 66: strcat(jniEntryName, cname);
>>> 67: }
>>
>> I would prefer this be directly inlined at the sole call (in findJniFunction),
>> to make it easier to verify there aren't any buffer overrun problems. (I don't
>> think there are, but looking at this in isolation triggered warnings in my
>> head.)
>>
>> Also, it looks like all callers of findJniFunction ensure the cname argument
>> is non-null. So there should be no need to handle the null case in
>> findJniFunction (other than perhaps an assert or something). That could be
>> addressed in a followup. (I've already implicitly suggested elsewhere in this
>> PR revising this function in a followup because of the JNI_ON[UN]LOAD_SYMBOLS
>> thing.)
>
> @kimbarrett I added this to https://bugs.openjdk.org/browse/JDK-8343703. You are not as explicit here as the other places you commented that it is okay to do as a follow-up, but I'll assume that was what you meant. If not, let me know, and I'll look at fixing it for this PR already.
The first part, eliminating the (IMO not actually helpful) helper function, I wanted done here. The second part,
cleaning up or commenting the calculation of the length and dealing with perhaps unneeded conditionals, I'm
okay with being in a followup. I guess I can live with the first part being in that followup too.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21744#discussion_r1831728737
More information about the core-libs-dev
mailing list