On Tue, 8 Mar 2022 03:51:03 GMT, David Holmes <dhol...@openjdk.org> wrote:
>> Johannes Bechberger has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Move WX functionality into os specific files > > src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.hpp line 57: > >> 55: static void current_thread_init_wx(); >> 56: >> 57: static void current_thread_assert_wx_state(WXMode expected); > > Can we move all these into the ThreadWXEnable class so they are not in the os > namespace? Even the enum could move - though it will make the use-sites a bit > more verbose. I won't insist on pushing this WX stuff even deeper, but if > anyone else thinks it is a good idea ... :) I'm open for suggestions, but putting it there was the simplest way. The problem is that os is not a namespace, but a class. But this could and should probably be changed. > src/hotspot/share/prims/jni.cpp line 97: > >> 95: #include "utilities/macros.hpp" >> 96: #include "utilities/vmError.hpp" >> 97: #include "runtime/thread.inline.hpp" > > Why do we need this? Why do we not include os.hpp? We need this, because it does not compile (linker error) otherwise. But I forgot to include os.hpp (but included by thread.inline.hpp). ------------- PR: https://git.openjdk.java.net/jdk/pull/7727
