RFR: 8341793: Fix ExceptionOccurred in jdk.accessibility

Laurent Bourgès lbourges at openjdk.org
Wed Nov 13 20:06:36 UTC 2024


On Wed, 13 Nov 2024 19:19:14 GMT, Phil Race <prr at openjdk.org> wrote:

>> Use ExceptionCheck instead
>
> 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

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21979#discussion_r1841089074


More information about the client-libs-dev mailing list