Why is this binary output code causing a memory leak

Question

I am new to C++ and I am trying to output a wave file in. I have had success with binary files in both C# and Java but I am not yet comfortable with C++ yet. I know about that arrays and objects should generally be created on the heap.

It is fine with the strings and the first getter

Whenever it gets to the second getter for the base class it runs out of memory. This class is called waveWriter and it extends a class called WaveFormat that contains the getters

WaveWriter header:

class WaveWriter : WaveFormat {
private:
    std::string fileName;

public:
    WaveWriter(uint16_t channels, uint32_t sampleRate,
            uint16_t bitsPerSample, double* lSampleData,
            uint32_t lSampleLength, double* rSampleData,
            uint32_t rSampleLength, bool isFloat, std::string outputName);

    ~WaveWriter();

    void writeWave() {

        std::ofstream myFile;
        myFile = std::ofstream(fileName, std::ios::out | std::ios::binary);

        // write the header
        // sGroupID
        myFile << S_GROUP_ID_HEADER;
        // dwfilelength
        myFile.write(reinterpret_cast<const char *> (GetDwFileLength()),
                sizeof (GetDwFileLength()));
        // sRiffType
        myFile << S_RIFF_TYPE;
        // write the format
        // sGroupID
        myFile << S_GROUP_ID_FORMAT;
        // dwChunkSize
        myFile.write(reinterpret_cast<const char *> (GetDwFormatChunkSize()),
                sizeof (GetDwFormatChunkSize()));
        // wFormatTag
        ........ blah blah blah

        // close file
        myFile.close();

        return;
    }
};

cpp for this class:

WaveWriter::WaveWriter(uint16_t channels, uint32_t sampleRate,
        uint16_t bitsPerSample, double* lSampleData,
        uint32_t lSampleLength, double* rSampleData,
        uint32_t rSampleLength, bool isFloat, std::string outputName) :
WaveFormat(channels, sampleRate, bitsPerSample,
lSampleData, lSampleLength, rSampleData,
rSampleLength, isFloat) {
    outputName.append(".wav");
    this->fileName = outputName;
}

WaveWriter::~WaveWriter() {
    this->~WaveFormat();
}

Header for WaveFormat contains private variables a constructor and getters like these to access the private variables:

public:
    uint16_t GetCbSize() {
        return cbSize;
    }

    uint32_t GetDwAvgBytesPerSec() {
        return dwAvgBytesPerSec;
    }

    uint32_t GetDwChannelMask() {
        return dwChannelMask;
    }......

Please provide a [mre] which demonstrates your reasons for assuming that there is a memory leak.# — Yunnosch, Jun 29 '21 at 08:44
`this->~WaveFormat();` -don't do that, see https://stackoverflow.com/questions/677620/do-i-need-to-explicitly-call-the-base-virtual-destructor — , Jun 29 '21 at 08:50
I get this message in a popup "The instruction at (hex memory address) referenced memory at (other memory addresss). The memory could not be read. — Edward Eddy67716, Jun 29 '21 at 08:50
A memory leak is when you are not deallocating all the memory and the memory usage grows over time. A crash is not that. What you are seeing appears to be a crash. Please update the question with a factual description of the behaviour you are observing in as much detail as you can. — , Jun 29 '21 at 08:55
@EdwardEddy67716 That's not running out of memory, that's using an invalid pointer — user253751, Jun 29 '21 at 09:00

Afshin · Accepted Answer · 2021-06-29T09:07:52.303

0

This is speculation based on your functions name, but I think this code:

myFile.write(reinterpret_cast<const char *> (GetDwFileLength()),sizeof (GetDwFileLength()));

is incorrect. Assuming GetDwFileLength() return size as value, it is incorrect to cast it to const char *. You need to save it in another argument and post its address to cast. Something like this:

auto val = GetDwFileLength();
myFile.write(reinterpret_cast<const char *> (&val), sizeof (val));

I see similar mistake several times in your code. This mistake can make invalid memory access.

In addition you should use virtual base destructor rather than calling base destructor from derived class. Never call base destructor in derived class.

edited Jun 29 '21 at 09:07

answered Jun 29 '21 at 08:51

Afshin

8,839
1
18
53

I was afraid this would be the answer. – Edward Eddy67716 Jun 29 '21 at 09:00
You should simply not call base destructor from derived class. Ever. It happens automatically so if you write it yourself you are doing it twice. – user253751 Jun 29 '21 at 09:00
So how do I call the destructor from the derived class? – Edward Eddy67716 Jun 29 '21 at 09:21
@EdwardEddy67716 you don't. define base class destructor as `virtual`. and don't manually call it in derived class. it will be automatically called. – Afshin Jun 29 '21 at 10:57

Why is this binary output code causing a memory leak

1 Answers1