Cannot create new wchar_t *

Question

I have this operator overloader. My program crashes at the creation of the new wchar_t array.

myObject &operator += (const myObject &s) {
    wchar_t *cat = wcscat(data, s.data);
    int len = wcslen(cat);
    wchar_t *test = new wchar_t[len + 1]; //this is killing!
    wcscpy(test, cat);

    delete data;
    data = test;

    return *this;
}

Does anybody know what's happening?

EDIT complete class definition

class myObject
{
    private:
        wchar_t *data;
    public:
        myObject() { data = 0; }
        ~myObject() { delete data; }

        myObject &operator += (const myObject &s) {
            wchar_t *cat = wcscat(data, s.data);
            int len = wcslen(cat);
            wchar_t *test = new wchar_t[len + 1];
            wcscpy(test, cat);

            delete data;
            data = test;

            return *this;
        }
};

Couldn't you save yourself a lot of trouble by having the class hold an `std::wstring`? — juanchopanza, Nov 10 '12 at 14:50
Is `data` big enough to hold `s.data` on the end? You're also invoking UB by using the wrong `delete`. — chris, Nov 10 '12 at 14:51
@chris my program hasn't been able to get past the creation of that new array. So it hasn't have anything to do with the object right? — HansElsen, Nov 10 '12 at 14:54
How are you calling the operator? If you say `x +=x;` then the call to `wscat` will result in undefined behaviour. — juanchopanza, Nov 10 '12 at 14:55
@HansElsen, Undefined behaviour can do what it wants, including crashing somewhere else. — chris, Nov 10 '12 at 14:59

score 3 · Accepted Answer · answered Nov 10 '12 at 14:56

3

This code contains, at least, two rather obvious problems:

You allocate data apparently using new wchar_t[n] but you release it using delete p rather than using delete[] p.
The likely cause of your problem is that you concatenate two strings into the memory of one string and then allocate enough memory to copy the data over.

You probably want something more along the lines of this:

myObject &operator += (const myObject &s) {
    size_t len = wcslen(this->data) + wcslen(s.data);
    std::unique_ptr<wchar_t[]> tmp(new wchar_t[len + 1]);
    wcscpy(tmp.get(), this->data);
    wcscat(tmp.get(), s.data);
    delete[] this->data;
    this->data = tmp.release();
    return *this;
}

Actually, I think you want to use std::wstring: this class already provides the logic, probably in a more efficient form anyway.

answered Nov 10 '12 at 14:56

Dietmar Kühl

150,225
13
225
380

1

"probably in a more efficient form" - for one thing, it won't have to measure the lengths of the strings with `wcslen`, or skip over the destination string like `wcscat` does. – Steve Jessop Nov 10 '12 at 15:00
Thanks for the answer. I will try it. My reason for not using the wstring is that I'm trying to learn to work with these arrays so I know whats happening – HansElsen Nov 10 '12 at 15:00
1

@HansElsen It's a good idea to learn what goes on behind the scenes. Once you've learned that don't forget to go back to doing things the easy way. – john Nov 10 '12 at 15:01
Thanks all, this solution worked. I also made a more basic solution with 2 for-loops to fill the new array. But the problem is clear now. From now on I can use wstrings haha. – HansElsen Nov 10 '12 at 15:12
You shouldn't use any `for`-loops for things like this, though! Ideally, you strings now how many characters they have and expose iterators (possibly to `wchar_t const`) to their elements. Filling the new array would then look something like this: `std::copy(s.begin(), s.end(), std::copy(this->begin(), this.end(), tmp.get()));` – Dietmar Kühl Nov 10 '12 at 15:21

Steve Jessop · Answer 2 · 2012-11-10T17:29:58.717

As Dietmar says (but with a bit more detail what probably happened behind the scenes):

1) The call wcscat(data, s.data) has overrun the end of the buffer pointed to by data. If it didn't overrun, then you wouldn't need to allocate a new buffer, since the existing one would be big enough. I expect it isn't big enough.

2) As a consequence of overrunning the buffer, data structures used by the memory allocator are trashed. This causes a crash when you try to allocate memory. It could just as easily cause a crash when you free memory, or never crash at all.

Cannot create new wchar_t *

2 Answers2