Do not take address of empty string front

Jonathan Wakely jwakely@redhat.com
Sat Jun 20 11:59:00 GMT 2015


On 20/06/15 12:03 +0200, François Dumont wrote:
>Hi
>
>    2 experimental tests are failing in debug mode because
>__do_str_codecvt is sometimes taking address of string front() and
>back() even if empty. It wasn't use so not a big issue but it still
>seems better to avoid. I propose to rather use string begin() to get
>buffer address.

But derefencing begin() is still undefined for an empty string.
Shouldn't that fail for debug mode too? Why change one form of
undefined behaviour that we diagnose to another form that we don't
diagnose?

It would be better if that function didn't do any work when the input
range is empty:

--- a/libstdc++-v3/include/bits/locale_conv.h
+++ b/libstdc++-v3/include/bits/locale_conv.h
@@ -58,6 +58,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
                     _OutStr& __outstr, const _Codecvt& __cvt, _State& __state,
                     size_t& __count, _Fn __fn)
     {
+      if (__first == __last)
+       {
+         __outstr.clear();
+         return true;
+       }
+
       size_t __outchars = 0;
       auto __next = __first;
       const auto __maxlen = __cvt.max_length() + 1;


>I kept operator& as it is what was used, do you think we
>should use addressof ?

I don't expect that function to get used with anything except built-in
character types (char, wchar_t, char16_t, char32_t) which don't use
ADL so can't find an overloaded operator&.

>    At the same time I improve string debug mode cause with front()
>implemented as operator[](0) it is ok even if string is empty while
>back() implemented as operator[](size()-1) is not which is quite
>inconsistent. So I added a number of !empty() checks in several methods
>to detect more easily all those invalid usages.

Nice. These would all be candidates for the "debug mode lite" that I
want.

>    * include/bits/basic_string.h (basic_string<>::front()): Add !empty
>    debug check.
>    (basic_string<>::back()): Likewise.
>    (basic_string<>::pop_back()): Likewise.
>    * include/bits/locale_env.h (__do_std_codecvt): Do not take address of

N.B. include/bits/locale_conv.h not locale_env.h



More information about the Libstdc++ mailing list