Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

<format>: Should we verify character arrays being null-terminated when creating basic_format_arg? #5094

Open
frederick-vs-ja opened this issue Nov 18, 2024 · 1 comment
Labels
enhancement Something can be improved format C++20/23 format

Comments

@frederick-vs-ja
Copy link
Contributor

There're Preconditions specified in [format.arg]/5 for constructing a basic_format_arg:

Preconditions: If decay_t<T> is char_type* or const char_type*, static_cast<const char_type*>(v) points to a NTCTS ([defns.ntcts]).

And character arrays are decayed per [format.arg]/6.9:

otherwise, if decay_t<TD> is char_type* or const char_type*, initializes value with static_cast<const char_type*>(v);

It seems that we can check the content of the array (of known bound) in this internal factory function.

// Function template _Make_from mirrors the exposition-only single-argument constructor template of
// basic_format_arg (N4950 [format.arg]).
template <_Formattable_with<_Context> _Ty>
_NODISCARD static basic_format_arg _Make_from(_Ty& _Val) noexcept {
using _Erased_type = _Format_arg_traits<_Context>::template _Storage_type<_Ty>;
if constexpr (is_same_v<remove_const_t<_Ty>, char> && is_same_v<_CharType, wchar_t>) {
return basic_format_arg(static_cast<_Erased_type>(static_cast<unsigned char>(_Val)));
}
#if !_HAS_CXX23
else if constexpr (is_same_v<_Erased_type, basic_string_view<_CharType>>) {
return basic_format_arg(_Erased_type{_Val.data(), _Val.size()});
}
#endif // !_HAS_CXX23
else {
return basic_format_arg(static_cast<_Erased_type>(_Val));
}
}

Should we verify that the array is null-terminated? Note that the checking is not O(1) but still seems cheap in formatting to me.

@frederick-vs-ja frederick-vs-ja added the question Further information is requested label Nov 18, 2024
@StephanTLavavej StephanTLavavej added enhancement Something can be improved format C++20/23 format and removed question Further information is requested labels Nov 20, 2024
@StephanTLavavej
Copy link
Member

We talked about this at the weekly maintainer meeting and we agree that this is worth doing.

We believe that this check can be performed without paying two linear-time costs. Here, we know we're dealing with char or wchar_t arrays, so there's no question of wacky char_traits. We can use strnlen/wcsnlen to get the length of the hopefully-null-terminated string but clamped to the array size. As the documentation explains, if these functions return the array size, that's impossible for a properly null-terminated string, so that's the error case that we can then precondition-check. As @CaseyCarter notes, we don't expect huge array arguments, and the additional expense should be low, so let's try an always-enabled check that uses _STL_VERIFY.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Something can be improved format C++20/23 format
Projects
None yet
Development

No branches or pull requests

2 participants