Calling delete[] on array of objects corrupts heap for no reason

Question

void Files::Push(const File& f)
{
    if(!s_)
    {
        files_ = new File[++s_];
        files_[0] = f;
        return;
    }

    File* tmp = new File[++s_];
    memcpy(tmp, files_, (s_-1) * sizeof(File));
    tmp[s_-1] = f;

    delete[] files_;

    files_ = tmp;
}

Full code is kind of big. But for this particular problem, its really simple code, really simple classes. I really don't see what could went wrong in here. I don't call delete[] twice, I don't call delete instead of delete[], I call it on same pointer that saved result of new[]. I thought, maybe some other code corrupts heap before this particular delete[], but I double checked my custom string class (only one that deals with raw pointers) and it works 100% correctly.

In full code you can see some debug outputs. It crashes after trying to scan 3rd file or 4th file in folder, randomly...

D:\Dropbox\My Programs\FileSysScanner>fss
D:\Dropbox\My Programs\FileSysScanner\fss.exe
D:\Dropbox\My Programs\FileSysScanner\fss.exe Size: 45
D:\Dropbox\My Programs\FileSysScanner
fss.exe
Scanning...: D:\Dropbox\My Programs\FileSysScanner\*
File Found: fss.exe
1
2
Inside new pointer if... s: 0
3
4
Scanning...: fss.exe\*
FindFirstFileW() Failed! Error# 267
5
File Found: fss_first_prot.exe
1
2
push_1... s: 1
push_2... s: 2
push_3... s: 2
push_4... s: 2
push_5... s: 2
3
4
Scanning...: fss_first_prot.exe\*
FindFirstFileW() Failed! Error# 267
5
File Found: Main.cpp
1
2
push_1... s: 2
push_2... s: 3
push_3... s: 3
push_4... s: 3
<CRASH HERE>

File class depicted below. Now I'm thinking mb including Files in File was not the best idea... Maybe this is the problem...

class File
{
public:
    File();
    File(const wchar_t* n, ulong b, bool isf, bool hid, bool sys);

    const Fname& Name() const;
    ulong Bytes() const;
    void AddBytes(ulong b);
    bool IsFolder() const;
    bool IsHidden() const;
    bool IsSystem() const;

    void PushFile(const wchar_t* n, ulong b, bool isf, bool hid, bool sys);
    void PushFile(const File& f);

private:
    void SetBytes(ulong b);
    ulong BytesTaken(ulong b) const;
    // Data
    Fname fn_;
    Files fs_;
    // Folder, Hidden and System attributes stored here in upper bits
    ulong bytes_;
    ulong bytes_taken_;
};

Edit:

I fixed it with making proper deep copy constructor and assignment for Files and also for File too.

Files::Files(const Files& other) : s_(other.s_)
{
    files_ = new File[s_];
    for(int i = 0; i < s_; ++i)
    {
        files_[i] = other.files_[i];
    }
}

Files& Files::operator=(const Files& other)
{
    if(this != &other)
    {
        File* tmp = files_;
        if(other.s_ != s_)
        {
            s_ = other.s_;
            tmp = new File[s_];
        }

        for(int i = 0; i < s_; ++i)
        {
            tmp[i] = other.files_[i];
        }

        delete[] files_;
        files_ = tmp;
    }

    return *this;
}

And also using simple loop instead of memcopy in PushFile():

void Files::Push(const File& f)
{
    if(!s_)
    {
        files_ = new File[++s_];
        files_[0] = f;
        return;
    }
    File* tmp = new File[++s_];

    for(int i = 0; i < s_-1; ++i)
    {
        tmp[i] = files_[i];
    }

    tmp[s_-1] = f;

    delete[] files_;
    files_ = tmp;
}

Now, using Files in File is not a problem at all. Same as using raw pointers (if you know what are you doing, ofcourse). If I didn't try to oversimplify thing with this stupid memcopy usage on non-POD classes, all would be fine...

Thanks for help!

Answer 1

my custom string class (only one that deals with raw pointers)

Your Files class has a raw pointer so that's bugged. Your File class includes a Files object so that's wrong as well.

memcpy on non-POD types (such as File) is another bug.

Don't use raw pointers, or if you insist then make sure your classes have correct copying semantics. Don't memcpy objects, use copy constructors and assignment. The easy and efficient way to write your code would be to use std::wstring and std::vector.