stdcxx-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Travis Vitek" <tvi...@quovadx.com>
Subject RE: [PATCH] Add overflow checking to basic_string append and push_back
Date Thu, 20 Sep 2007 22:06:32 GMT
 

Martin Sebor wrote:
>
>Travis Vitek wrote:
>> It appears that recent changes to string have accidentally 
>> removed some overflow checking that used to be in the
>> basic_string::append() and push_back() methods. The following
>> patch adds the checks back in.
>
>Does this fix a test failure? Or some regression? (If the former,
>which test? If the latter, we need a test case :)

This is a regression for which we don't have a testcase.

>
>In any event, adding an if to these two performance sensitive
>functions looks risky from an efficiency POV (we'd definitely
>need to see before and after timings to consider the patch).
>

Yes. I agree.

>
>The change also seems unnecessary -- when size() equals capacity()
>we check that it doesn't exceed max_size() before allocating more
>memory in append(). Otherwise, when size() is less than capacity()
>(or rather capacity() - 1), there should be no reason to check
>against max_size() because we know that capacity() must have
>been below max_size() the last time we reallocated.

If that is the case, then why would we possibly need this same code in
any of the other methods that are used to extend the original string?
Here is a testcase.

#include <string>
#include <stdexcept>
#include <new>

template <class _TypeT> class
Xallocator;

template <>
class Xallocator<void>
{
public:
    typedef void              value_type;
    typedef value_type*       pointer;
    typedef const value_type* const_pointer;
   
    template <class _TypeU> 
    struct rebind {
        typedef Xallocator<_TypeU> other;
    };    
};

template <class _TypeT>
class Xallocator
{
public:
    typedef unsigned char       size_type;
    typedef signed char         difference_type;
    typedef _TypeT              value_type;
    typedef value_type*         pointer;
    typedef const value_type*   const_pointer;
    typedef value_type&         reference;
    typedef const value_type&   const_reference;

    Xallocator () {
    }

    Xallocator (const Xallocator &) {
    }

    template <class _TypeU> 
    struct rebind {
        typedef Xallocator<_TypeU> other;
    };

    template <class _TypeU>
    Xallocator (const Xallocator<_TypeU>&) {
    }

    template <class _TypeU>
    Xallocator&
    operator= (const Xallocator<_TypeU>&) { 
        return *this; 
    }

    pointer address (reference __x) const {
        return _RWSTD_REINTERPRET_CAST (pointer,
                   &_RWSTD_REINTERPRET_CAST (char&, __x));
    }

    const_pointer address (const_reference __x) const {
        return _RWSTD_REINTERPRET_CAST (const_pointer,
                   &_RWSTD_REINTERPRET_CAST (const char&, __x));
    }

    pointer allocate (size_type __n, Xallocator<void>::const_pointer =
0) {
        return (pointer)::operator new (__n * sizeof (value_type));
    }

    void deallocate (pointer __p, size_type) {
        ::operator delete (__p);
    }

    // 20.4.1.1, p11 - the largest N for which allocate (N) might
succeed
    size_type max_size () const { 
        return 255;
    }

    void construct (pointer __p, const_reference __val) {
        new (__p) value_type(__val);
    }
    
    void destroy (pointer __p) {
        __p->~_TypeT ();
    }
};

typedef std::basic_string<char, std::char_traits<char>, Xallocator<char>
> String;

static void test_append_1()
{
    const String x(1, 'x');

    String a;
    for (int n = 0; n < 1024; ++n)
        a.append(x);
}

static void test_append_2()
{
    const String x(1, 'x');

    String a;
    for (int n = 0; n < 1024; ++n)
        a.append(x, 0, 1);
}

static void test_append_3()
{
    String a;
    for (int n = 0; n < 1024; ++n)
        a.append("x", 1);
}

static void test_append_4()
{
    String a;
    for (int n = 0; n < 1024; ++n)
        a.append("x");
}

static void test_append_5()
{
    static const char x[] = "x";

    String a;
    for (int n = 0; n < 1024; ++n)
        a.append(x, x+1);
}

static void test_push_back()
{
    String a;
    
    // this should throw, but i get heap corruption
    for (int n = 0; n < 1024; ++n)
        a.push_back ('a');
}


int main ()
{
    int failed = 0;

    void (*tests[])() = {
        &test_append_1,
        &test_append_2,
        &test_append_3,
        &test_append_4,
        &test_append_5,
        &test_push_back,
        0 // sentinel
    };

    for (int n = 0; tests[n]; ++n)
    {
        try {
            tests[n]();

            failed += 1;
        }
        catch (const std::length_error&) {
            // expected
        }
        catch (...) {
            failed += 1;
        }
    }

    return failed;
}


Mime
View raw message