13650 – string::compare should not (always) use traits_type::length() (original) (raw)

| Description Jacques Labuschagne 2004-01-12 04:13:33 UTC The following function is incorrect: template<typename _CharT, typename _Traits, typename _Alloc> int basic_string <_CharT, _Traits, _Alloc>:: compare(size_type __pos, size_type __n1, const _CharT* __s, size_type __n2) const The implementation should only be trying to calculate the length of __s when __n2 is npos, because __s may have '\0' characters embedded in it; i.e. it may not be a NTBS. The use of traits_type::length is still appropriate when __n2 is npos. Proposed solution: --- basic_string.tcc.old 2004-01-12 17:12:57.000000000 +1300 +++ basic_string.tcc 2004-01-12 17:13:01.000000000 +1300 @@ -1033,7 +1033,9 @@ if (__pos > __size) __throw_out_of_range("basic_string::compare"); - size_type __osize = std::min(traits_type::length(__s), __n2); + size_type __osize = __n2; + if (__n2 == npos) + __osize = traits_type::length(__s); size_type __rsize = std::min(size_type(__size - __pos), __n1); size_type __len = std::min(__rsize, __osize); int __r = traits_type::compare(_M_data() + __pos, __s, __len); Comment 1 Jacques Labuschagne 2004-01-12 04:39:06 UTC This should also mean that we can unify the two similar comparisons into one which uses a default 4th argument (as appears in the standard) - These two compare(size_type __pos, size_type __n1, const _CharT* __s, size_type __n2) const compare(size_type __pos, size_type __n1, const _CharT* __s) const become compare(size_type __pos, size_type __n1, const _CharT* __s, size_type __n2 = npos) const Comment 2 Drea Pinski 2004-01-12 05:39:51 UTC From the C++ standard (21.3.6.8): int compare(size_type pos, size_type n1, charT *s, size_type n2 = npos) const; 6 Returns: basic_string<charT,traits,Allocator>(*this,pos,n1).compare( basic_string<charT,traits,Allocator>(s,n2)) So I agree that they should be combined (they are not on the mainline). Comment 4 Paolo Carlini 2004-01-12 21:04:51 UTC Hi. There are a few issues, here, and the proposed patch is incorrect. 1- The standard had a defect, DR 5 (TC) (see also DR 87), http://anubis.dkuug.dk/jtc1/sc22/wg21/docs/lwg-defects.html#5 indeed, one of the very first discovered, which implies that we _cannot_ unify the two different compare as you are suggesting. 2- The version missing the fourth parameter is ok as-is. 3- The version with the fourth size_type parameter should be tweaked, however, I'm working on it. Basically (see also Josuttis, p. 512), '\0' has no special meaning and s _must_ have at least n2 chars: therefore __osize == __n2, always. Thanks, Paolo. P.S. I have also checked two other well known implementations and both are consistent with 1-, 2-, 3- above. Comment 6 Paolo Carlini 2004-01-13 11:17:40 UTC Fixed for 3.4. | | | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | |