<i18n dev> RFR: 8342682: Errors related to unused code on Windows after 8339120 in dt_shmem jdwp security and jpackage
Julian Waters
jwaters at openjdk.org
Wed Oct 23 05:26:05 UTC 2024
On Tue, 22 Oct 2024 18:03:12 GMT, Chris Plummer <cjplummer at openjdk.org> 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
>
> src/jdk.jdi/windows/native/libdt_shmem/shmem_md.c line 47:
>
>> 45: {
>> 46: void *mappedMemory;
>> 47: // HANDLE memHandle;
>
> Why comment out this one but not the one at line 88? It seems they are both equally problematic and are hiding the static memHandle. I'm not sure why the 2nd one isn't flagged. I'd actually suggest getting rid of the static memHandle. It does not seem to be needed.
I wasn't sure whether the global memHandle not being used was a bug, so I commented out the local one. I missed the line 88 one because it wasn't flagged. If it really isn't needed I'll remove that one instead
> src/jdk.jdwp.agent/share/native/libjdwp/log_messages.c line 53:
>
>> 51: #ifndef _WIN32
>> 52: static MUTEX_T my_mutex = MUTEX_INIT;
>> 53: #endif
>
> The reason for no reference on windows is because of the following on windows:
>
>
> #define MUTEX_LOCK(x) /* FIXUP? */
> #define MUTEX_UNLOCK(x) /* FIXUP? */
>
>
> So looks like this mutex support is something we never got around to. I think your current workaround is fine.
I'm curious now, how to implement mutex support on Windows? I think I prefer that to just making it completely unavailable on Windows
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21616#discussion_r1811884490
PR Review Comment: https://git.openjdk.org/jdk/pull/21616#discussion_r1811885815
More information about the i18n-dev
mailing list