On Mon, 19 Feb 2024 13:04:47 GMT, Matthias Baesken <[email protected]> wrote:
> In AccessBridgeJavaEntryPoints.cpp we have a couple of exception checks with
> potential early returns. Those miss ReleaseStringChars .
src/jdk.accessibility/windows/native/libjavaaccessbridge/AccessBridgeJavaEntryPoints.cpp
line 1747:
> 1745: length = jniEnv->GetStringLength(js);
> 1746: EXCEPTION_CHECK_WITH_RELEASE("Getting AccessibleName - call
> to GetStringLength()", FALSE, js, stringBytes);
> 1747: EXCEPTION_CHECK("Getting AccessibleName - call to
> ReleaseStringChars()", FALSE);
So this is for the call to ReleaseStringChars() when there is no exception
doing the GetStringLength
Since all the calls to ReleaseStringChars are now moved inside the WITH_RELEASE
macro, it would seem more
logical to me to move this 2nd macro call inside the definition of the new
macro (option 1)
Either that OR (option 1) do not move the "no previous exception" case inside
the new macro at all
In other words, this CHECK should be placed next to where I can see the call
that requires it.
Now, you'll probably tell me it isn't needed in the cases where the method
immediately returns, so if that causes
a problem then option 2 seems like it would be better.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17915#discussion_r1498381660
