Search code examples
c++methodsmemory-leaksfree

How to avoid this memory leak?


This is my code:

void MIDITest::CreateNoteBlock() {
    IMidiMsgExt* midiMessage = new IMidiMsgExt;
    midiMessage->MakemidiMessageMsg(57, 100, 0, 0, 0);
    queuedNotes.insert(*midiMessage);

    midiMessage = new IMidiMsgExt;
    midiMessage->MakemidiMessageMsg(60, 100, 0, tickSize * 38, 0);
    queuedNotes.insert(*midiMessage);

    midiMessage = new IMidiMsgExt;
    midiMessage->MakemidiMessageMsg(62, 100, 0, 0, 0);
    queuedNotes.insert(*midiMessage);

    midiMessage = new IMidiMsgExt;
    midiMessage->MakemidiMessageMsg(65, 100, 0, tickSize * 32, 0);
    queuedNotes.insert(*midiMessage);

    midiMessage = new IMidiMsgExt;
    midiMessage->MakemidiMessageMsg(57, 0, tickSize * 111, 0);
    queuedNotes.insert(*midiMessage);

    midiMessage = new IMidiMsgExt;
    midiMessage->MakemidiMessageMsg(60, 0, tickSize * 111, 0);
    queuedNotes.insert(*midiMessage);

    midiMessage = new IMidiMsgExt;
    midiMessage->MakemidiMessageMsg(62, 0, tickSize * 75, 0);
    queuedNotes.insert(*midiMessage);

    midiMessage = new IMidiMsgExt;
    midiMessage->MakemidiMessageMsg(65, 0, tickSize * 105, 0);
    queuedNotes.insert(*midiMessage);
}   

so at every new operator it will allocate a block of memory.

Should I use free after any insert inside the queuedNotes? Or it will be released after void function return? (i.e. the brackets of CreateNoteBlock).

Or I can "reuse" each time the midiMessage pointer for a new IMidiMsgExt?


Solution

  • The answer is not to use new at all. Create a object with automatic storage duration

    IMidiMsgExt midiMessage;
    

    And then you can keep calling MakemidiMessageMsg and insert a copy of the message into the multiset.

    midiMessage.MakemidiMessageMsg(57, 100, 0, 0, 0);
    queuedNotes.insert(midiMessage);
    midiMessage.MakemidiMessageMsg(60, 100, 0, tickSize * 38, 0);
    queuedNotes.insert(midiMessage);
    //...
    

    Now the multiset has a copy of all of the messages and at the end of the function midiMessage is destroyed and no memory management needs to be done.

    If IMidiMsgExt has a constructor that is like MakemidiMessageMsg where you can construct a complete message then you could boild this down even further and use something like

    queuedNotes.insert(IMidiMsgExt(57, 100, 0, 0, 0));
    queuedNotes.insert(IMidiMsgExt(60, 100, 0, tickSize * 38, 0));
    

    And now we don't even need midiMessage.