RFR: 8342870: Errors related to unused code on Windows after 8339120 in accessibility [v5]

[Julian Waters]

On Tue, 21 Jan 2025 21:18:27 GMT, Alexey Ivanov <aivanov at openjdk.org> wrote:

>> Julian Waters has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains eight commits:
>> 
>>  - Merge branch 'openjdk:master' into accessibility
>>  - Cast to void in AccessBridgeCalls.c
>>  - static_cast to void in jaccessinspector.cpp
>>  - Formatting changes in AccessBridgeEventHandler.cpp
>>  - Merge branch 'master' into accessibility
>>  - Remove now unused result
>>  - Merge branch 'master' into accessibility
>>  - 8342870
>
> src/jdk.accessibility/windows/native/bridge/AccessBridgeCalls.c line 230:
> 
>> 228:             if (result != TRUE) {
>> 229:                 error = GetLastError();
>> 230:             }
> 
> The comparison above should be `if (result == 0)` instead of `!= TRUE`, because [`FreeLibrary`](https://learn.microsoft.com/en-us/windows/win32/api/libloaderapi/nf-libloaderapi-freelibrary) returns non-zero if the function succeeds, which implies the return value could be different from 1 (`TRUE`).

Fixed. I also changed the logic below to return FALSE if the FreeLibrary call above failed

> src/jdk.accessibility/windows/native/jaccessinspector/jaccessinspector.cpp line 1212:
> 
>> 1210:     }
>> 1211: 
>> 1212:     static_cast<void>(lastError);
> 
> This falls into category that's useful for debugging only. The value of `lastError` is never used to report an error, yet one can use a debugger to inspect the value.
> 
> There seems to be no way to report or log the error.
> 
> Isn't it better to add a flag to suppress the warning? Casting `lastError` to `void` is cryptic. A comment would definitely be helpful here.

I suppose I could do that yes, if that that is preferred

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21656#discussion_r1926387441
PR Review Comment: https://git.openjdk.org/jdk/pull/21656#discussion_r1926388732