On Tue, 31 Oct 2023 07:32:35 GMT, Kim Barrett <kbarr...@openjdk.org> wrote:
>> I know he was away until October but not exactly when in October. > > 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) ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/15955#discussion_r1377224772
