Search code examples
c++binaryfiles

Why is this binary output code causing a memory leak


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;
    }......

Solution

  • 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.