Что способствует нарушению кучи?

Question

[CodeInside]

При вызове деструктора объекта p (дальше в коде увидите) выдаёт ошибку 'Debug Assertion Failed. Expression: _CrtIsValidHeapPointer(pUserData). Я так понял, где-то утечка памяти, или некорректная работа с динамической памятью, но никак не могу определить в чём ошибка.

Минимальный вариант кода:

class Path
{
    char* path;

public:
    Path(char*);
    Path(const Path &);
    Path(Path &&);
    ~Path();

    Path operator + (const Path &) const;
    Path& operator = (const Path &);
    Path& operator = (Path &&);
}

Path::Path()
{
    path = new char[strlen("C:\\") + 1];
    strcpy_s(path, strlen("C:\\") + 1, "C:\\");
}

Path::Path(char* path)
{
    this->path = new char[strlen(path) + 1];
    strcpy_s(this->path, strlen(path) + 1, path);
}

Path::~Path()
{
    delete[] path;
    path = nullptr;
}

Path::Path(const Path &obj)
{
    path = new char[strlen(obj.path) + 1];
    strcpy_s(path, strlen(obj.path) + 1, obj.path);
}

Path::Path(Path &&obj)
{
    path = obj.path;
    obj.path = nullptr;
}

void main()
{
    Path p1("C:\\Program Files\\"), p2("Google Chrome\\Cache");
    Path p = p1 + p2;
}

Полная версия:

#include <iostream>
#include <io.h>
#include <conio.h>
using namespace std;
 
class Path
{
    char* path;
 
public:
    Path();
    Path(char*);
    Path(const Path &);
    Path(Path &&);
    ~Path();
 
    int SetPath(char*);
    char* GetPath() const;
    bool Exists() const;
    bool Exists(char* path, bool overwrite_this_path = false);
    bool IsFile() const;
    bool IsDirectory() const;
    bool IsFullPath() const;
 
    Path operator + (const char &) const;
    Path operator + (const Path &) const;
    Path& operator += (const char &);
    Path& operator += (const Path &);
    Path& operator = (const char &);
    Path& operator = (const Path &);
    Path& operator = (Path &&);
 
private:
    //  Removes extra slashes
    //  Example: (p1 == 'D:\\Data\\', p2 == 'Front-end\\Messages.json').
    //  User wrote 'path = p1 + '\\' + p2'.
    //  Then path == 'D:\\Data\\\\Front-end\\Messages.json'.
    //  This method converts this path in view like 'D:\\Data\\Front-end\\Messages.json'
    void CheckSlashes();   
};
 
Path::Path()
{
    path = new char[strlen("C:\\") + 1];
    strcpy_s(path, strlen("C:\\") + 1, "C:\\");
}
 
Path::Path(char* path)
{
    this->path = new char[strlen(path) + 1];
    strcpy_s(this->path, strlen(path) + 1, path);
    CheckSlashes();
}
 
Path::~Path()
{
    delete[] path;
    path = nullptr;
}
 
Path::Path(const Path &obj)
{
    path = new char[strlen(obj.path) + 1];
    strcpy_s(path, strlen(obj.path) + 1, obj.path);
    CheckSlashes();
}
 
Path::Path(Path &&obj)
{
    path = obj.path;
    obj.path = nullptr;
    CheckSlashes();
}
 
int Path::SetPath(char* path)
{
    if (path == nullptr)
    {
        return 1;
    }
 
    delete[] path;
    this->path = new char[strlen(path) + 1];
 
    if (this->path == nullptr)
    {
        return 2;
    }
 
    strcpy_s(this->path, strlen(path) + 1, path);
    CheckSlashes();
 
    return 0;
}
 
char* Path::GetPath() const
{
    return path;
}
 
bool Path::Exists() const
{
    _finddata_t data;
    int descriptor = _findfirst(path, &data);
 
    if (descriptor == -1)
    {
        return false;
    }
    else
    {
        return true;
    }
}
 
bool Path::Exists(char* path, bool overwrite_this_path)
{
    if (path == nullptr)
    {
        return false;
    }
 
    if (overwrite_this_path)
    {
        SetPath(path);
    }
 
    _finddata_t data;
    int descriptor = _findfirst(path, &data);
 
    if (descriptor == -1)
    {
        return false;
    }
    else
    {
        return true;
    }
}
 
bool Path::IsFile() const
{
    _finddata_t data;
    int descriptor = _findfirst(path, &data);
 
    if (descriptor == -1)
    {
        return false;
    }
 
    if (!(data.attrib & _A_SUBDIR))
    {
        return true;
    }
    else
    {
        return false;
    }
}
 
bool Path::IsDirectory() const
{
    if (path == nullptr)
    {
        return false;
    }
 
    _finddata_t data;
    int descriptor = _findfirst(path, &data);
 
    if (descriptor == -1)
    {
        return false;
    }
 
    if (data.attrib & _A_SUBDIR)    // data is directory?
    {
        return true;
    }
    else
    {
        return false;
    }
}
 
bool Path::IsFullPath() const
{
    bool IsFullPath = false;
    if (strlen(path) > 1)   // Directory name may consist of one character with
    {
        IsFullPath = (path[1] == ':');  // Example: 'C:\\Program Files', 'D:\\Data\\Source.cpp'
    }
    return IsFullPath;
}
 
Path& Path::operator = (const char &ch)
{
    delete[] path;
 
    path = new char[1];
    path[0] = ch;
 
    CheckSlashes();
    return *this;
}
 
Path& Path::operator = (const Path &pth)
{
    if (this == &pth)
    {
        return *this;
    }
 
    delete[] path;
 
    path = new char[strlen(pth.path) + 1];
    strcpy_s(path, strlen(pth.path) + 1, pth.path);
 
    CheckSlashes();
    return *this;
}
 
Path Path::operator + (const char &ch) const
{
    Path obj;
 
    int buff_size = strlen(this->path) + 2; // 2 - @ch and '\0'
    obj.path = new char[buff_size];
 
    strcpy_s(obj.path, buff_size, this->path);
 
    obj.path[strlen(this->path)] = ch;
    obj.path[strlen(this->path) + 1] = '\0';
 
    return obj;
}
 
// XXX: '\\' beetwen paths (NOT TESTED!)
Path Path::operator + (const Path &pth) const
{
    Path obj;
 
    int buff_size = strlen(this->path) + strlen(pth.path) + 1;
    obj.path = new char[buff_size];
 
    // Example: 'C:\Program Files' + 'Google Chrome\Cache'
    // @obj.path must be 'C:\Program Files\Google Chrome\Cache', not 'C:\Program FilesGoogle Chrome\Cache'
    strcpy_s(obj.path, buff_size, this->path);
    if (obj.path[strlen(obj.path) - 1] != '\\' && pth.path[0] != '\\')
    {
        obj += '\\';
        buff_size++;
    }
 
    // Example: 'C:\Program Files\' + '\Google Chrome\Cache'
    // @obj.path must be 'C:\Program Files\Google Chrome\Cache', not 'C:\Program Files\\Google Chrome\Cache'
    if (obj.path[strlen(obj.path) - 1] == '\\' && pth.path[0] == '\\')
    {
        obj.path[strlen(obj.path) - 1] = '\0';
    }
   
    strcat_s(obj.path, buff_size, pth.path);
 
    return obj;
}
 
Path& Path::operator += (const char &ch)
{
    char* tmp = new char[strlen(path) + 1];
    strcpy_s(tmp, strlen(path) + 1, path);
 
    delete[] path;
    path = new char[strlen(tmp) + 2];   // 2 - @ch and '\0'
 
    strcpy_s(path, strlen(tmp) + 2, tmp);
 
    path[strlen(tmp)] = ch;
    path[strlen(tmp) + 1] = '\0';
 
    CheckSlashes();
    return *this;
}
 
Path& Path::operator += (const Path &pth)
{
    char* tmp = new char[strlen(path) + 1];
    strcpy_s(tmp, strlen(path) + 1, path);
 
    delete[] path;
    int buff_size = strlen(tmp) + strlen(pth.path) + 1;
    path = new char[buff_size];
 
    strcpy_s(path, buff_size, tmp);
    strcat_s(path, buff_size, pth.path);
 
    CheckSlashes();
    return *this;
}
 
Path& Path::operator = (Path &&pth)
{
    delete[] path;
    path = pth.path;
    pth.path = nullptr;
    CheckSlashes();
 
    return *this;
}
 
void Path::CheckSlashes()
{
    char* ptr = strstr(path, "\\\\");
    while(ptr != nullptr)
    {
        for (int i = 0; i < strlen(ptr); i++)
            ptr[i] = ptr[i + 1];
        ptr = strstr(path, "\\\\");
    }
}
 
void main()
{
    Path p1("C:\\Program Files"), p2("Google Chrome\\Cache");
    Path p = p1 + p2;
}

Comments

[CodeInside]

Ivan Sokolov: мы ещё не учили класс string

Answer 1

[Петр]

Тут

Path Path::operator + (const Path &pth) const
{
   Path obj;
 
    int buff_size = strlen(this->path) + strlen(pth.path) + 1;
    obj.path = new char[buff_size]; // утекло то, что в конструкторе без параметров создано
    strcpy_s(obj.path, buff_size, this->path); // количество символов для копирования больше чем в this->path
    ...
}

Comments

[CodeInside]

Я добавил строку 'delete[] obj.path' после обьявления 'Path obj' в методах 'perator + (const char &ch)' и 'operator + (const Path&ch)'. Но ошибка так и осталась. Что именно не так в строке 'strcpy_s(obj.path, buff_size, this->path)'? Буфер слишком мал для конкатенации? Вроде всё отлично отрабатывал ...

[Петр]

CodeInside: strlen(this->path) < buff_size

[CodeInside]

Ну ещё и в семантику переноса добавил 'delete[] path' на всякий.

[CodeInside]

Петр: но ведь второй параметр, принимаемый функцией strcpy_s,- размер буфера строки, в которую копируем. Если я в второй параметр передам 'strlen( this->path)', то получу ошибку "Buffer is too small".

[Петр]

CodeInside: С этим вы правы. Далее у вас идет obj += '\\'; внутри которого вы делаете delete[] path; и затем возращаясь назад делаете strcat_s(obj.path, buff_size, pth.path); где уже нету выделенных buff_size

[CodeInside]

Петр: я понял, но не могу придумать как эффективно закрыть этот баг. Перераспределением буффера после конкатенации символа? И вопрос: почему всё-таки выделенный после вызова оператора '+=' буффер, не сохраняется (ведь это метод, который неявно получает указатель на объект, и выполняет всё напрямую)?

[Петр]

CodeInside: Замените '+=' на strcat_s, только учтите, что strlen(this->path) + strlen(pth.path) + 1 не подразумевает наличие этого дополнительного символа. ))

[CodeInside]

Петр: Благодарю.