This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Do not take address of empty string front


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]