RFR: 8341793: Fix ExceptionOccurred in jdk.accessibility
Phil Race
prr at openjdk.org
Wed Nov 13 20:14:57 UTC 2024
On Wed, 13 Nov 2024 20:02:49 GMT, Laurent Bourgès <lbourges at openjdk.org> wrote:
>> src/jdk.accessibility/windows/native/libjavaaccessbridge/AccessBridgeJavaEntryPoints.cpp line 78:
>>
>>> 76:
>>> 77: #define EXCEPTION_CHECK(situationDescription, returnVal) \
>>> 78: if (exception = jniEnv->ExceptionOccurred()) { \
>>
>> So what about this variable exception that is being assigned ?
>> The bug is supposed to be about when the return value of ExceptionOccurred() isn't used.
>> Now you aren't assigning it either (1) some code will be broken, unless (2) it isn't actually used
>> If (2) then you would want to delete the declaration of exception.
>> But if it is used *even once* then you need to keep all of them and revert this change.
>>
>> I did a grep on the file and I think it is unused, so you'll need to remove them all
>> % grep exception windows/native/libjavaaccessbridge/AccessBridgeJavaEntryPoints.cpp
>> ...
>> if (exception = jniEnv->ExceptionOccurred()) { \
>> if (exception = jniEnv->ExceptionOccurred()) { \
>> if (exception = jniEnv->ExceptionOccurred()) { \
>> if (exception = jniEnv->ExceptionOccurred()) { \
>> jthrowable exception;
>> jthrowable exception;
>> jthrowable exception;
>> jthrowable exception;
>> jthrowable exception;
>> jthrowable exception;
>> jthrowable exception;
>> jthrowable exception;
>> jthrowable exception;
>> jthrowable exception;
>> jthrowable exception = NULL;
>> jthrowable exception;
>> jthrowable exception;
>> jthrowable exception;
>> jthrowable exception;
>> ... and a whole lot more.
>
> I will check you're right.
> It looked suspicious, no real use of exception, I will look again
I suspect all those declarations exist only because the macros need it.
So removing them will be the right solution
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21979#discussion_r1841101315
More information about the client-libs-dev
mailing list