RFR: 8307160: Fix AWT/2D/A11Y to support the permissive- flag on the Microsoft Visual C compiler [v46]

[Julian Waters]

On Wed, 20 Mar 2024 06:22:50 GMT, Julian Waters <jwaters at 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