2016-04-21 5 views
0

Это мой код:Как избежать утечки памяти?

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

так у каждого new оператора будет выделять блок памяти.

Должен ли я использовать free после любых insert внутри queuedNotes? Или он будет выпущен после возврата void? (т. е. скобки CreateNoteBlock).

Или я могу «повторно использовать» каждый раз, когда указатель midiMessage для нового IMidiMsgExt?

+0

Покажите нам полный код. Что такое 'queuedNotes'? – Chiel

+0

@Chiel это 'multiset queuedNotes;'. Это код: он вызывается внутри CTOR класса. – markzzz

+4

Как насчет использования 'IMidiMsgExt midiMessage;' вместо 'IMidiMsgExt * midiMessage = новый IMidiMsgExt'? –

ответ

6

Ответ: не использовать new. Создание объекта с длительностью

IMidiMsgExt midiMessage; 

автоматического хранения И тогда вы можете продолжать звонить MakemidiMessageMsg и вставить копию сообщения в multiset.

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

Теперь multiset имеет копию всех сообщений, и в конце функции midiMessage разрушается и не управление памятью должно быть сделано.

Если IMidiMsgExt имеет конструктор, который, как MakemidiMessageMsg, где вы можете построить полное сообщение, то вы можете boild это еще дальше и использовать что-то вроде

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

И теперь мы даже не нужно midiMessage.

+0

Но в конце функции не будет также уничтожен «все» midiMessage (созданный с помощью нового)? – markzzz

+0

@paizza No. Если вы вызываете 'new', единственный способ освободить память - вызвать' delete'. Указатель уничтожается, но память автоматически не возвращается. – NathanOliver

+0

Это связано с автоматической продолжительностью хранения? Итак, ваше предложение о 'автоматической длительности хранения',' new' о 'динамической длительности хранения' http://stackoverflow.com/questions/6337294/creating-an-object-with-or-without-new – markzzz

5

Вы исходите из фона Java или C#? В C++ вы не должны использовать new создавать объекты, просто объявляя их будет работать нормально:

IMidiMsgExt midiMessage; 
midiMessage.MakemidiMessageMsg(57, 100, 0, 0, 0); 
queuedNotes.insert(midiMessage); 

Это либо, что (это мое рекомендуемое решение), или вы должны явно освободить объекты:

IMidiMsgExt* midiMessage = new IMidiMsgExt; 
midiMessage->MakemidiMessageMsg(57, 100, 0, 0, 0); 
queuedNotes.insert(*midiMessage); 
delete miniMessage; 
+0

Да, я программист на C#. – markzzz

1

Кажется излишним динамически выделять IMidiMsgExt используя новые. Вы можете просто выделить его непосредственно в стеке (без указателей), а затем он будет уничтожен, когда ваш метод вернется. I.e .:

void MIDITest::CreateNoteBlock() { 
    IMidiMsgExt midiMessage(); 
    midiMessage.MakemidiMessageMsg(57, 100, 0, 0, 0); 
    queuedNotes.insert(midiMessage); 

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

    midiMessage = IMidiMsgExt(); 
    midiMessage.MakemidiMessageMsg(62, 100, 0, 0, 0); 
    queuedNotes.insert(midiMessage); 

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

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

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

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

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

Попробуйте сделать свой API более похожим на C++.

Используйте один объект в стеке вместо того, чтобы создавать много новых в куче.

void MIDITest::CreateNoteBlock() { 
    IMidiMsgExt midiMessage; 

    midiMessage.MakemidiMessageMsg(57, 100, 0, 0, 0); 
    queuedNotes.insert(midiMessage); 

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

    // ... 
} 

Инициализировать объекты в конструкторе. Определите operator = (const IMidiMsgExt&) для IMidiMsgExt.

void MIDITest::CreateNoteBlock() { 
    IMidiMsgExt midiMessage(57, 100, 0, 0, 0); 
    queuedNotes.insert(midiMessage); 

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

    // ... 
} 

Я думаю, что insert() ожидает const IMidiMsgExt&. Таким образом, вы можете напрямую передавать только что инициализированные объекты:

void MIDITest::CreateNoteBlock() { 
    queuedNotes.insert({57, 100, 0, 0, 0}); 
    queuedNotes.insert({60, 100, 0, tickSize * 38, 0}); 

    // ... 
} 

BTW: вы должны предпочесть использовать, например,std::queue<> для queuedNotes. Тогда вы не будете использовать insert(), но push(), или emplace(). Преимущество emplace() в том, что он создает объект в контейнере, а не первым его создания, а затем скопировать его в контейнер:

void MIDITest::CreateNoteBlock() { 
    queuedNotes.emplace(57, 100, 0, 0, 0); 
    queuedNotes.emplace(60, 100, 0, tickSize * 38, 0); 

    // ... 
} 

Также ваши TYPENAME IMidiMsgExt сигналы для меня, что вы пытаетесь имитировать C# мышление в C++. Это возможно, но обычно это не предпочтительное решение. Из вашего вопроса я не знаю достаточно о вашем дереве классов и основных требованиях для предоставления предложения, но на C++, который обычно является запахом кода.

Смежные вопросы