On Tue, 7 Jan 2025 09:56:11 GMT, Julian Waters <[email protected]> wrote:
>> After 8339120, gcc began catching many different instances of unused code in
>> the Windows specific codebase. Some of these seem to be bugs. I've taken the
>> effort to mark out all the relevant globals and locals that trigger the
>> unused warnings and addressed all of them by commenting out the code as
>> appropriate. I am confident that in many cases this simplistic approach of
>> commenting out code does not fix the underlying issue, and the warning
>> actually found a bug that should be fixed. In these instances, I will be
>> aiming to fix these bugs with help from reviewers, so I recommend anyone
>> reviewing who knows more about the code than I do to see whether there is
>> indeed a bug that needs fixing in a different way than what I did
>>
>> build.log on release configuration:
>>
>> C:/users/vertig0/downloads/eclipse-committers-2023-12-r-win32-x86_64/workspace/jdk/src/jdk.accessibility/windows/native/jabswitch/jabswitch.cpp:
>> In function 'int regEnable()':
>> C:/users/vertig0/downloads/eclipse-committers-2023-12-r-win32-x86_64/workspace/jdk/src/jdk.accessibility/windows/native/jabswitch/jabswitch.cpp:334:14:
>> warning: unused variable 'freeData' [-Wunused-variable]
>> 334 | bool freeData = false;
>> | ^~~~~~~~
>> C:/users/vertig0/downloads/eclipse-committers-2023-12-r-win32-x86_64/workspace/jdk/src/jdk.accessibility/windows/native/jabswitch/jabswitch.cpp:326:11:
>> warning: unused variable 'retval' [-Wunused-variable]
>> 326 | DWORD retval = -1;
>> | ^~~~~~
>> C:/users/vertig0/downloads/eclipse-committers-2023-12-r-win32-x86_64/workspace/jdk/src/jdk.accessibility/windows/native/jabswitch/jabswitch.cpp:
>> In function 'int regDeleteValue(HKEY, LPCWSTR)':
>> C:/users/vertig0/downloads/eclipse-committers-2023-12-r-win32-x86_64/workspace/jdk/src/jdk.accessibility/windows/native/jabswitch/jabswitch.cpp:379:14:
>> warning: unused variable 'freeData' [-Wunused-variable]
>> 379 | bool freeData = false;
>> | ^~~~~~~~
>> C:/users/vertig0/downloads/eclipse-committers-2023-12-r-win32-x86_64/workspace/jdk/src/jdk.accessibility/windows/native/jabswitch/jabswitch.cpp:367:11:
>> warning: unused variable 'retval' [-Wunused-variable]
>> 367 | DWORD retval = -1;
>> | ^~~~~~
>> C:/users/vertig0/downloads/eclipse-committers-2023-12-r-win32-x86_64/workspace/jdk/src/jdk.accessibility/windows/native/bridge/AccessBridgeCalls.c:
>> In function 'shutdownAccessBridge':
>> C:/users/vertig0/downloads/eclipse-committers-2023-12-r-win3...
>
> 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 233:
> 231: return TRUE;
> 232: }
> 233: ((void) error);
Not sure why do we need it. I mean there is a value assignment before so why
would compiler complain?
src/jdk.accessibility/windows/native/jabswitch/jabswitch.cpp line 326:
> 324: int regEnable() {
> 325: HKEY hKey;
> 326: // DWORD retval = -1;
I don't think we need to comment out both retval and freeData. It is either
remnants of the code that was long gone or (more likely) were just a copy of
another template method that actually uses these variables. If variables are
not used within the function i would just delete them.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21656#discussion_r1922905467
PR Review Comment: https://git.openjdk.org/jdk/pull/21656#discussion_r1922905577
