On Tue, 22 Oct 2024 18:03:12 GMT, Chris Plummer <cjplum...@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

Reply via email to