Dynamische Zuweisung verliert Speicher?

Question

bool CPythonNonPlayer::LoadNonPlayerData(const char *c_szFileName) 
{ 
    DWORD dwElements; 

    TMobTable *pTable = (TMobTable *) zObj.GetBuffer(); 

    for(DWORD i = 0; i < dwElements; ++i, ++pTable) 
    { 
     TMobTable *pNonPlayerData = new TMobTable; 
     memcpy(pNonPlayerData, pTable, sizeof(TMobTable)); 
     m_NonPlayerDataMap.insert(TNonPlayerDataMap::value_type(pNonPlayerData->dwVnum, pNonPlayerData)); 
    } 

    return true; 
} 

Meine Frage ist: was mache ich falsch? Dies führt zu einer Menge Speicherverlust. Nach jedem Aufruf dieser Funktion erhöht sich die Anwendungsnutzung um 10 MB.

Answer 1

Das Problem ist nicht in dieser Funktion. Das Problem liegt in der Art, wie Sie mit m_NonPlayerDataMap umgehen. Diese Funktion überträgt den Besitz einiger Objekte auf diese Karte, und die Verantwortung für diese Karte liegt bei delete, wenn sie damit erledigt ist. Ich wette, das tut es nicht.

Übrigens, um diese Art von Problem zu vermeiden tun Sie das einfach nicht. Verwenden Sie nicht new, es sei denn Sie müssen wirklich. Erstellen Sie stattdessen eine Karte der Werte statt einer Karte von Zeigern. Wenn Sie keine Möglichkeit finden, dies zu erreichen, sollten Sie zumindest intelligente Zeiger anstelle von rohen Zeigern verwenden.

Answer 2

Verwenden Smart-Pointer-Wrapper Speicherverwaltung für Sie zu behandeln, zum Beispiel:

Bei Verwendung einer älteren Version als C++ 11:

#include <memory> 

// std::auto_ptr is not container-safe! 
typedef std::map<DWORD, TMobTable*> TNonPlayerDataMap; 
TNonPlayerDataMap m_NonPlayerDataMap; 

... 

bool CPythonNonPlayer::LoadNonPlayerData(const char *c_szFileName) 
{ 
    DWORD dwElements = ...; 
    ... 

    // I'm assuming this just returns a pointer to an existing memory 
    // buffer and is not actually allocating a new buffer. If it is, 
    // you need to free it when you are done copying it... 
    // 
    TMobTable *pTable = (TMobTable *) zObj.GetBuffer(); 

    for(DWORD i = 0; i < dwElements; ++i, ++pTable) 
    { 
     std::auto_ptr<TMobTable> pNonPlayerData(new TMobTable); 

     // don't use memcpy! better would be to give TMobTable a copy constructor instead... 
     // std::auto_ptr<TMobTable> pNonPlayerData(new TMobTable(*pTable)); 
     // 
     *pNonPlayerData = *pTable; 

     // if successful, release local ownership of the object. 
     // if failed, ownership will remain here and free the object when the auto_ptr goes out of scope. 
     // 
     if (m_NonPlayerDataMap.insert(std::make_pair(pNonPlayerData->dwVnum, pNonPlayerData.get())).second) 
      pNonPlayerData.release(); 
    } 

    return true; 
} 

Alternativ, wenn Sie mit C++ 11 oder später:

#include <memory> 

// std::unique_ptr is container-safe! 
typedef std::map<DWORD, std::unique_ptr<TMobTable>> TNonPlayerDataMap; 
TNonPlayerDataMap m_NonPlayerDataMap; 

... 

bool CPythonNonPlayer::LoadNonPlayerData(const char *c_szFileName) 
{ 
    DWORD dwElements = ...; 
    ... 

    // I'm assuming this just returns a pointer to an existing memory 
    // buffer and is not actually allocating a new buffer. If it is, 
    // you need to free it when you are done copying it... 
    // 
    TMobTable *pTable = (TMobTable *) zObj.GetBuffer(); 

    for(DWORD i = 0; i < dwElements; ++i, ++pTable) 
    { 
     std::unique_ptr<TMobTable> pNonPlayerData(new TMobTable); 
     // 
     // or, if using C++14 or later: 
     // std::unique_ptr<TMobTable> pNonPlayerData = std::make_unique<TMobTable>(); 

     // don't use memcpy! better would be to give TMobTable a copy constructor instead... 
     // std::unique_ptr<TMobTable> pNonPlayerData(new TMobTable(*pTable)); 
     // std::unique_ptr<TMobTable> pNonPlayerData = std::make_unique<TMobTable>(*pTable); 
     // 
     *pNonPlayerData = *pTable; 

     // if successful, ownership of the object is transferred into the map. 
     // if failed, ownership will remain here and free the object when the unique_ptr goes out of scope. 
     // 
     m_NonPlayerDataMap.insert(std::make_pair(pNonPlayerData->dwVnum, std::move(pNonPlayerData))); 
    } 

    return true; 
}