RFR: 8317132: Prepare HotSpot for permissive- [v10]

Thomas Stuefe stuefe at openjdk.org
Wed Nov 1 07:52:04 UTC 2023


On Tue, 31 Oct 2023 08:37:44 GMT, Julian Waters <jwaters at openjdk.org> wrote:

>> Ick! I hadn't followed https://github.com/openjdk/jdk/pull/15096 closely, so
>> hadn't noticed the discussion there.  Not sure why this is using a literal 0
>> rather than `(T) '\n'` as in that PR?
>> 
>> So the literal "0" (or previously, NUL char constant) is either an integral
>> zero or a null pointer constant (because one of the types used for T is
>> HMODULE). imprint_sentinal uses `(T)'X'`. (Double ick! The value is either an
>> integral constant or a HMODULE with a weird value.)
>> 
>> An approach to being type safe/consistent would be to provide a traits utility
>> for specifying the terminator and the sentinal on a type-specific basis. Don't
>> know if that's worth doing in this PR. We've been living (dangerously?) with
>> the existing mechanism for a long time.
>> 
>> Something like
>> 
>> template<typename T>
>> struct SimpleBufferSpecialValues;
>> 
>> template<>
>> struct SimpleBufferSpecialValues<char> {
>>   char terminator() const { return '\0'; }
>>   char sentinal() const { return '\X'; }
>> };
>> 
>> template<>
>> struct SimpleBufferSpecialValues<HMODULE> {
>>   HMODULE terminator() const { return nullptr; }
>>   HMODULE sentinal() const {
>>     // Untested, might be completely wrong.
>>     alignas(HMODULE) static const char sentinal_data[1];
>>     return reinterpret_cast<HMODULE>(&sentinal_data);
>>   }
>> };
>
> I'd felt that the literal zero fit better, because it avoided an ambiguous cast and could fit in the context of a null pointer constant and char value of 0 both, casting a char constant to a pointer seemed to be something to avoid. The HMODULE or char in imprint_sentinel would always have a value of 88, which I'm not too fond of either (particularly in the case of HMODULE, which doesn't seem right since 88 is a perfectly normal value for a HMODULE to have, doesn't seem to fit the role of sentinel value too well)

Wow, I cannot recall ever getting bashed this hard for code of mine.

Feel free to remove the Sentinel mechanism completely. It's not needed. I would keep the zero termination of the array.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15955#discussion_r1378486602


More information about the build-dev mailing list