On Wed, 20 Mar 2024 06:22:50 GMT, Julian Waters <jwat...@openjdk.org> wrote:
>> src/java.desktop/windows/native/libawt/windows/awt_Component.cpp line 6366:
>>
>>> 6364: jobject parent = data->parentComp;
>>> 6365:
>>> 6366: AwtComponent *awtComponent = nullptr;
>>
>> Looking at it (not tested) here through 6403 could be simplified as
>>
>> if (self == NULL || parent == NULL) {
>> env->ExceptionClear();
>> JNU_ThrowNullPointerException(env, "peer");
>> env->DeleteGlobalRef(self);
>> env->DeleteGlobalRef(parent);
>> delete data;
>> return;
>> }
>>
>> AwtComponent *awtComponent = (AwtComponent *)JNI_GET_PDATA(self);
>> if (awtComponent == NULL) {
>> THROW_NULL_PDATA_IF_NOT_DESTROYED(self);
>> env->DeleteGlobalRef(self);
>> env->DeleteGlobalRef(parent);
>> delete data;
>> return;
>> }
>>
>> AwtComponent *awtParent = (AwtComponent *)JNI_GET_PDATA(parent);
>> if (awtParent == NULL) {
>> THROW_NULL_PDATA_IF_NOT_DESTROYED(parent);
>> env->DeleteGlobalRef(self);
>> env->DeleteGlobalRef(parent);
>> delete data;
>> return;
>> }
>>
>>
>> I think I prefer that over
>> BOOL error = FALSE;
>> AwtComponent *awtComponent = nullptr;
>> AwtComponent *awtParent = nullptr;
>>
>> if (self == NULL || parent == NULL) {
>> env->ExceptionClear();
>> JNU_ThrowNullPointerException(env, "peer");
>> error = TRUE;
>> }
>>
>> if (!error) {
>> awtComponent = (AwtComponent *)JNI_GET_PDATA(self);
>> if (awtComponent == NULL) {
>> THROW_NULL_PDATA_IF_NOT_DESTROYED(self);
>> error = TRUE;
>> }
>> }
>>
>> if (!error) {
>> awtParent = (AwtComponent *)JNI_GET_PDATA(parent);
>> if (awtParent == NULL) {
>> THROW_NULL_PDATA_IF_NOT_DESTROYED(parent);
>> error = TRUE;
>> }
>>
>> if (error) {
>> env->DeleteGlobalRef(self);
>> env->DeleteGlobalRef(parent);
>> delete data;
>> return;
>> }
>
> Sorry, I don't see a BOOL error anywhere?
I see the advantage of collapsing self and parent into the same check, but it
doesn't seem like getting rid of pData is of much benefit, the checks for null
seem to remain the same either way
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15096#discussion_r1531567293
