On Thu, 17 Mar 2022 16:23:14 GMT, Tyler Steele <d...@openjdk.java.net> wrote:

>> As described in the linked issue, NullClassBytesTest fails due an 
>> OutOfMemoryError produced on AIX when the test calls defineClass with a byte 
>> array of size of 0. The native implementation of defineClass then calls  
>> malloc with a size of 0. On AIX malloc(0) returns NULL, while on other 
>> platforms it return a valid address. When NULL is produced by malloc for 
>> this reason, ClassLoader.c incorrectly interprets this as a failure due to a 
>> lack of memory.
>> 
>> ~~This PR modifies ClassLoader.c to produce an OutOfMemoryError only when 
>> `errno == ENOMEM` and to produce a ClassFormatError with the message 
>> "ClassLoader internal allocation failure" in all other cases (in which 
>> malloc returns NULL).~~ [edit: The above no longer describes the PR's 
>> proposed fix. See discussion below]
>> 
>> In addition, I performed some minor tidy-up work in ClassLoader.c by 
>> changing instances of `return 0` to `return NULL`, and `if (some_ptr == 0)` 
>> to `if (some_ptr == NULL)`. This was done to improve the clarity of the code 
>> in ClassLoader.c, but didn't feel worthy of opening a separate issue.
>> 
>> ### Alternatives
>> 
>> It would be possible to address this failure by modifying the test to accept 
>> the OutOfMemoryError on AIX. I thought it was a better solution to modify 
>> ClassLoader.c to produce an OutOfMemoryError only when the system is 
>> actually out of memory.
>> 
>> ### Testing
>> 
>> This change has been tested on AIX and Linux/x86.
>
> Tyler Steele has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Improve comment
>   
>   - Reword to avoid double use of malloc(X)
>   - Remove bug id

src/java.base/share/native/libjava/ClassLoader.c line 104:

> 102:     // On AIX malloc(0) returns NULL which looks like an out-of-memory 
> condition; so adjust it to malloc(1)
> 103:     #ifdef _AIX
> 104:     body = (jbyte *)malloc(length == 0 ? 1 : length);

Can we use identification in the ifdef/else/endif block to make it a bit more 
readable. Also can you trim down the comment or split it over two lines to 
avoid the really long line (it makes it a bit easier for future side-by-side 
reviews).

-------------

PR: https://git.openjdk.java.net/jdk/pull/7829

Reply via email to