On Thu, 12 Oct 2023 09:33:24 GMT, Joachim Kern <jk...@openjdk.org> wrote:

>> src/java.base/aix/native/libjava/ProcessHandleImpl_aix.c line 89:
>> 
>>> 87: 
>>> 88:     do { // Block to break out of on Exception
>>> 89:         pids = (*env)->GetLongArrayElements(env, jarray, NULL);
>> 
>> Nit, I'd move these invocations of GLAE into the initialization block and 
>> remove the outer loop. I see what you do here, the outer block gives you a 
>> reliable jumping point to get cleanup in case of an error. But I would just 
>> use a goto cleanup label. That's clearer and allows you to handle 
>> initialization in one place.
>> 
>> Proposal for initialization:
>> 
>> 
>> if (jparentArray != null) {
>>   - check size is same as primary array, if wrong throw + goto cleanup
>>   - ppids = GLAE
>> } else {
>>   - if input pid is 0, throw an error too, since for "get all processes mode"
>>       we need the parent array. Throw + goto cleanup.
>> }
>> // same for times array
>
> hmm? As I already mentioned I do not want to change the structure of the 
> function with this pr. This can be done by someone else for macos, linux and 
> aix in parallel.

If someone reworks the code, you would have to counter-check the code anyway 
since nobody else has AIX machines around to build and test. So I'd just do it 
to save some cycles. But I leave it up to you.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16051#discussion_r1356614666

Reply via email to