Re: libstdc++/7961: compare( char *) implemented incorrectly.

[John Carter <john.carter@tait.co.nz>]

The following reply was made to PR libstdc++/7961; it has been noted by GNATS.

From: John Carter <john.carter@tait.co.nz>
To: Andreas Schwab <schwab@suse.de>
Cc: john.carter@tait.co.nz, gcc-gnats@gcc.gnu.org
Subject: Re: libstdc++/7961: compare( char *) implemented incorrectly.
Date: Thu, 19 Sep 2002 09:27:14 +1200 (NZST)

 If we going to do two comparisons and a subtract why not
 
    template<typename _CharT, typename _Traits, typename _Alloc>
      int
      basic_string<_CharT, _Traits, _Alloc>::
      compare(const _CharT* __s) const
      {
        size_type __size = this->size();
        size_type __s_size = traits_types::length(__s);
 
        if (__size == __s_size) 
           return traits_type::compare(_M_data(), __s, __min);
 
        size_type __min = __size;
        int __result = -1;
        if ( __size  > __s_size) {
          __result = 1;
          __min = __s_size;
        }
  
        int __r = traits_type::compare(_M_data(), __s, __min);
        if (__r)
          return __r;
 
        return __result;
      }
 
 Hmm, I'm not sure that is better, probably need to look at the
 assembler generated or benchmark it. 
 
 Even faster would be to fold in an implementation of strcmp. (We
 already scan the c-string once with the trait_types::length,
 cosmically speaking we shouldn't need to do that.). 
 
 Unfortunately, just using strcmp directly is probably a bad idea as
 you have to take care. A string can have null's in any place but a c
 string can only have it at the end. 
 
  ie. 
    string abcNull( "abc\0");
 
    abcNull > "abc" 
    strcmp( abcNull.c_str(), "abs") == 0
 
 I would go for correctness and simplicity over speed.
 
    
 
 On Wed, 18 Sep 2002, Andreas Schwab wrote:
 
 > john.carter@tait.co.nz writes:
 > 
 > |> A correct implementation would be...
 > |>   template<typename _CharT, typename _Traits, typename _Alloc>
 > |>     int
 > |>     basic_string<_CharT, _Traits, _Alloc>::
 > |>     compare(const _CharT* __s) const
 > |>     {
 > |>       size_type __size = this->size();
 > |>       size_type __s_size = traits_types::length(__s);
 > |>       size_type __min = __size;
 > |>       if ( __size  > __s_size) 
 > |>         __min = __s_size;
 > |> 
 > |>       int __r = traits_type::compare(_M_data(), __s, __min);
 > |>       if (!__r)
 > |> 	__r = __size - _s_size;
 > |>        
 > |>       return __r;
 > |>     }
 > 
 > This is not correct either, because __size - __s_size may overflow the
 > range of int.  Try this instead:
 > 
 >       if (!__r)
 > 	__r = (__size > __s_size) - (__size < __s_size);
 > 
 > Andreas.
 > 
 > 
 
 -- 
 
 
 John Carter                             Phone : (64)(3) 358 6639
 Tait Electronics                        Fax   : (64)(3) 359 4632
 PO Box 1645 Christchurch                Email : john.carter@tait.co.nz
 New Zealand
 
 Good Ideas:
 Ruby                 - http://www.ruby-lang-org - The best of perl,python,scheme without the pain.
 Valgrind             - http://developer.kde.org/~sewardj/ - memory debugger for x86-GNU/Linux
 Free your books      - http://www.bookcrossing.com