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