Использование delete [] (повреждение кучи) при реализации оператора + = - PullRequest
2 голосов
/ 02 мая 2010

Я пытаюсь понять это часами, и я нахожусь в конце. Я, безусловно, был бы признателен, если бы кто-то мог сказать мне, когда я делаю что-то не так.

Я написал простой класс для эмуляции базовой функциональности строк. Члены класса включают в себя символьный указатель data (который указывает на динамически создаваемый массив символов) и целое число strSize (которое содержит длину строки, без терминатора.)

Поскольку я использую new и delete , я реализовал конструктор копирования и деструктор. Моя проблема возникает, когда я пытаюсь реализовать оператор + = . Объект LHS создает новую строку правильно - я даже могу напечатать ее с помощью cout - но проблема возникает, когда я пытаюсь освободить указатель данных в деструкторе: я получаю «Обнаружено повреждение кучи после нормального блока» по адресу памяти, указанному на с помощью массива data деструктор пытается освободить его.

Вот мой полный курс и тестовая программа:

#include <iostream>

using namespace std;

// Class to emulate string
class Str {
public:

    // Default constructor
    Str(): data(0), strSize(0) { }

    // Constructor from string literal
    Str(const char* cp) {
        data = new char[strlen(cp) + 1];
        char *p = data;
        const char* q = cp;
        while (*q)
            *p++ = *q++;
        *p = '\0';
        strSize = strlen(cp);
    }

    Str& operator+=(const Str& rhs) {
        // create new dynamic memory to hold concatenated string
        char* str = new char[strSize + rhs.strSize + 1];

        char* p = str;                  // new data
        char* i = data;                 // old data
        const char* q = rhs.data;       // data to append

        // append old string to new string in new dynamic memory
        while (*p++ = *i++) ;
        p--;
        while (*p++ = *q++) ;
        *p = '\0';

        // assign new values to data and strSize
        delete[] data;
        data = str;
        strSize += rhs.strSize;
        return *this;
    }


    // Copy constructor
    Str(const Str& s)
    {
        data = new char[s.strSize + 1];
        char *p = data;
        char *q = s.data;
        while (*q)
            *p++ = *q++;
        *p = '\0';
        strSize = s.strSize;
    }

    // destructor
    ~Str() { delete[] data;  }

    const char& operator[](int i) const { return data[i]; }
    int size() const { return strSize; }

private:
    char *data;
    int strSize;
};

ostream& operator<<(ostream& os, const Str& s)
{
    for (int i = 0; i != s.size(); ++i)
        os << s[i];
    return os;
}


// Test constructor, copy constructor, and += operator
int main()
{
    Str s = "hello";        // destructor  for s works ok
    Str x = s;              // destructor for x works ok
    s += "world!";          // destructor for s gives error
    cout << s << endl;
    cout << x << endl;
    return 0;
}

РЕДАКТИРОВАТЬ : Ускоренная проблема C ++ 12-1.

Ответы [ 4 ]

4 голосов
/ 02 мая 2010

Следующий фрагмент кода делает p указанным рядом с массивом.

while (*p++ = *q++) ;
*p = '\0';

Лучшее (и безопасное) решение, которое вы использовали в конструкторе копирования:

while (*q)
    *p++ = *q++;
*p = '\0';
3 голосов
/ 03 мая 2010

Здесь уже есть куча хороших ответов, но стоит включить Valgrind в качестве инструмента для решения именно такой проблемы. Если у вас есть доступ к * nix box, инструменты Valgrind могут стать настоящим спасением.

Просто чтобы показать вам, вот что я получил при компиляции и запуске вашей программы через него:

% g++ -g -o test test.cpp
% valgrind ./test
==2293== Memcheck, a memory error detector
==2293== Copyright (C) 2002-2009, and GNU GPL'd, by Julian Seward et al.
==2293== Using Valgrind-3.5.0-Debian and LibVEX; rerun with -h for copyright info
==2293== Command: ./test
==2293==
==2293== Invalid write of size 1
==2293==    at 0x8048A9A: Str::operator+=(Str const&) (test.cpp:36)
==2293==    by 0x8048882: main (test.cpp:82)
==2293==  Address 0x42bc0dc is 0 bytes after a block of size 12 alloc'd
==2293==    at 0x4025024: operator new[](unsigned int) (vg_replace_malloc.c:258)
==2293==    by 0x8048A35: Str::operator+=(Str const&) (test.cpp:26)
==2293==    by 0x8048882: main (test.cpp:82)
==2293== 
helloworld!
hello
==2293== 
==2293== HEAP SUMMARY:
==2293==     in use at exit: 0 bytes in 0 blocks
==2293==   total heap usage: 4 allocs, 4 frees, 31 bytes allocated
==2293== 
==2293== All heap blocks were freed -- no leaks are possible
==2293== 
==2293== For counts of detected and suppressed errors, rerun with: -v
==2293== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 17 from 6)
%

Вы можете видеть, что он точно определил строки, на которые указывали другие ответы (около строки 36).

2 голосов
/ 02 мая 2010
while (*p++ = *i++) ; // the last iteration is when i is one past the end
// i is two past the end here -- you checked for a 0, found it, then incremented past it
p--; //here you corrected for this
while (*p++ = *q++) ;// the last iteration is when p and q are one past the end
// p and q are two past the end here
// but you didn't insert a correction here
*p = '\0';  // this write is in unallocated memory

Используйте идиому, аналогичную той, которую вы использовали в конструкторе копирования:

while (*i) *p++ = *i++; //in these loops, you only increment if *i was nonzero
while (*q) *p++ = *q++;
*p = '\0'
1 голос
/ 02 мая 2010

У вас уже есть два ответа, указывающих на конкретную ошибку, из-за которой вы выбросили кучу. Предполагая, что это домашнее задание или какое-либо другое упражнение (в противном случае мы все будем кричать на вас за написание вашего собственного класса струн), вот еще несколько вещей, которые нужно пережевать :

  • Если вы чувствуете необходимость аннотировать код, рассмотрите возможность сделать его более выразительным .
    Например, вместо char* p = str; // new data вы можете просто написать char* new_data = str;.
    Вместо //do frgl, за которым следует фрагмент кода, вы можете просто написать do_frgl();. Если функция встроенная, для результирующего кода это не имеет никакого значения, но большая разница для читателей кода.
  • Каждый, включая ваш заголовок, получает все из пространства имен std, сброшенное в глобальное пространство имен . Это вообще не очень хорошая идея. Я бы не стал включать ваш заголовок как чуму.
  • Ваши конструкторы должны инициализировать членов своего класса в списках инициализаторов .
  • Ваш конструктор Str::Str(const char*) дважды вызывает std::strlen() для одной и той же строки.
    Код приложения должен быть так быстро, как необходимо , код библиотеки , на с другой стороны, если вы не знаете, в каком приложении оно заканчивается, должно быть как можно быстрее . Ты пишешь библиотечный код.
  • Может ли функция-член size() вернуть отрицательное значение ? Если нет, то почему это целое число со знаком?
  • Что будет с этим кодом: Str s1, s2; s1=s2?
  • А что по этому поводу: Str str("abc"); std::cout<<str[1];

(Если кто-то сталкивался с этим, может подумать о большем количестве подсказок, не стесняйтесь расширять это.)

...