RFR: 8265473: Move os::Linux to its own header file [v4]

Ioi Lam iklam at openjdk.org
Mon Jul 11 03:32:45 UTC 2022


On Sat, 9 Jul 2022 23:27:31 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> Another step of moving unnecessary stuff outside of os.hpp
>> 
>> The `os::Linux` class is used only by the Linux-specific code in HotSpot. Therefore, it should be moved outside of os.hpp, which is used by platform-independent code.
>> 
>> I don't have a good name for the new header. `os_linux.hpp` would have been a good name, but that's already taken, so I am settling on os_linux.impl.hpp. Suggestions are welcome.
>
> Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:
> 
>   renamed to os_linux_impl.hpp

David and Thomas,

I think you're right. The current structure of the `os.hpp` header file doesn't make sense. Files like `os_linux.hpp` and `os_linux_x86.hpp` declare additional member functions of the `os` class. However, these functions are never (or should never be) used by cross platform code. For example,  `workaround_expand_exec_shield_cs_limit()` declared in `os_linux_x86.hpp` is used only under the src/hotspot/os/linux diretory.

My proposal is to remove all os- and platform- specific includes from `os.hpp`. The `os` class should include only functions that are usable from shared code.

Then, we can keep the `os_linux.hpp` file, which in turn would include other `os_linux_<arch>.hpp` files. These files should declare functions that are used by Linux-specific source files.

For naming, I would prefer to leave it as `os::Linux` for now, so as to avoid making a huge number of changes. We can change it to something else (like the ones suggested by Thomas) in a separate RFE.

Yes, it's legal to declare `os::Linux` outside of the declaration of the `os` class. See https://en.cppreference.com/w/cpp/language/nested_types

With my new proposal, the function `workaround_expand_exec_shield_cs_limit()` declared in `os_linux_x86.hpp` will move from `os::workaround_expand_exec_shield_cs_limit()` to `os::Linux::workaround_expand_exec_shield_cs_limit()`.

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

PR: https://git.openjdk.org/jdk/pull/9423


More information about the hotspot-dev mailing list