RFR: 8290840: Refactor the "os" class [v4]
Ioi Lam
iklam at openjdk.org
Tue Aug 2 07:04:04 UTC 2022
On Tue, 2 Aug 2022 06:53:49 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:
>> Ioi Lam has updated the pull request incrementally with four additional commits since the last revision:
>>
>> - clarify comment
>> - PLATFORM_PRINT_NATIVE_STACK -> HAVE_PLATFORM_PRINT_NATIVE_STACK
>> - simplified rarely-defined function os::resolve_function_descriptor()
>> - Made workaround_expand_exec_shield_cs_limit() a static function in os_linux.cpp
>
> Still good.
@tstuefe @gerard-ziemski
I've simplified `os::resolve_function_descriptor()` according to your suggestions, but I changed the #ifdefs to be more exact:
// Used only on PPC for AIX, or LINUX (ppc but not ppcle).
static void* resolve_function_descriptor(void* p)
#if defined(PPC) && (defined(AIX) || (defined(LINUX) && !defined(ABI_ELFv2)))
;
#else
{ return NULL; }
#endif
If we use "#if defined(PPC) && !defined(ABI_ELFv2)))" it would be very difficult to understand.
To be honest, I am not a big fan of adding #ifdefs into os.hpp just to save a few lines of code. The #ifdefs are hard to understand. They are also fragile if you need to add a new platform that also support this functionality.
I am (semi) OK with doing this for resolve_function_descriptor(), as that's a PPC-only thing that's unlikely to apply for other platforms. However, I don't want to use the same pattern for os::platform_print_native_stack() because we may want to implement this for other planforms.
-------------
PR: https://git.openjdk.org/jdk/pull/9600
More information about the hotspot-dev
mailing list