CodeGuru Home VC++ / MFC / C++ .NET / C# Visual Basic VB Forums Developer.com
Results 1 to 12 of 12

Thread: memory leak

  1. #1
    Join Date
    Nov 2003
    Posts
    24

    memory leak

    hi,guys,
    i has been puzzled with the following problem about memory leak in VC++6.anyone knows what results in the problem? thaz in advance.
    .h file

    class VolRec
    {
    public:
    VolRec(){
    m_wNumCharaList = 0;
    #ifdef _UNICODE
    m_pCharaList = NULL;
    #else
    m_pCharaList = NULL;

    #endif
    ...
    }
    virtual ~ VolRec(){
    if(m_pCharaList != NULL){
    delete [] m_pCharaList;
    m_pCharaList = NULL;
    }

    }
    VolRec operator = (const VolRec& vc){
    ...
    if(m_wNumCharaList!=0){
    #ifdef _UNICODE
    int num = m_wNumCharaList/2;
    m_pCharaList = new WCHAR[num+1];
    for(int i = 0; i <= num;i++ ){
    m_pCharaList[i] = vc.m_pCharaList[i];
    }
    #else
    int num = m_wNumCharaList;
    m_pCharaList = new char [num+1];
    for(int i = 0; i<= num; i++){
    m_pCharaList[i] = vc.m_pCharaList[i];
    }
    #endif
    }

    ...

    return *this;
    }
    VolRec(const VolRec& vc){
    ...
    if(m_wNumCharaList!=0){
    #ifdef _UNICODE
    int num = m_wNumCharaList/2;
    m_pCharaList = new WCHAR[num+1];
    for(int i = 0; i <= num;i++ ){
    m_pCharaList[i] = vc.m_pCharaList[i];
    }
    #else
    int num = m_wNumCharaList;
    m_pCharaList = new char [num+1];
    for(int i = 0; i<= num; i++){
    m_pCharaList[i] = vc.m_pCharaList[i];
    }
    #endif
    }

    ...

    }


    WORD m_wNumCharaList;


    #ifdef _UNICODE
    WCHAR* m_pCharaList;
    #else
    char* m_pCharaList;
    #endif

    };
    class KiwiIndexFrmData
    : public KiwiIndexFrm
    {
    public:

    KiwiIndexFrmData();
    virtual ~KiwiIndexFrmData();

    private:


    vector <VolRec*> m_vVolRec;

    protected:

    bool LoadVolRec(const LPBYTE pb,const DWORD& offset);
    };
    .cpp file
    KiwiIndexFrmData::KiwiIndexFrmData()
    {
    // ToDo: Add your specialized code here and/or call the base class
    }


    KiwiIndexFrmData::~KiwiIndexFrmData()
    {
    // ToDo: Add your specialized code here and/or call the base class

    int szVOLRec = m_vVolRec.size();
    if(szVOLRec!=0){
    for(int i = 0; i < szVOLRec; i++){

    delete m_vVolRec[i];

    }
    m_vVolRec.clear();
    }
    }


    bool KiwiIndexFrmData::LoadVolRec(const LPBYTE pb, const DWORD &offset)
    {

    bool bRet = true;


    for(int i = 0;i < ulNumVOLRec;i++ ){

    VolRec* vc = new VolRec;//KiwiIndexFrmData.cpp(320) memory leak
    ...
    #ifdef _UNICODE



    WORD num = vc->m_wNumCharaList/2; if(num!=0){
    vc->m_pCharaList = new WCHAR [num+1];//KiwiIndexFrmData.cpp(338) memory leak
    for(WORD i = 0; i < num; i++)
    {
    ::memcpy((void*)(vc->m_pCharaList+i),
    (void*)(pb+ofVOL+offlocal), sizeof(WCHAR));

    kpt.WORDInvert_per4bit(vc->m_pCharaList[i]);

    offlocal = offlocal + sizeof(WCHAR);

    }
    vc->m_pCharaList[num] = '\0';

    }

    #else
    WORD num = vc->m_wNumCharaList/2;
    WCHAR* pwc = new WCHAR [num+1];
    for(WORD i = 0; i < num; i++)
    {
    ::memcpy((void*)(pwc+i),
    (void*)(pb+ofVOL+offlocal), sizeof(WCHAR));

    kpt.WORDInvert_per4bit(pwc[i]);

    offlocal = offlocal + sizeof(WCHAR);

    }
    pwc[num] = '\0';
    vc->m_pCharaList = new char [2*num+1];
    ::WideCharToMultiByte(CP_ACP, 0,
    pwc,num+1,vc->m_pCharaList,2*(num+1),NULL,NULL);

    delete [] pwc;

    #endif

    ...

    m_vVolRec.push_back(vc);

    }

    return bRet;
    }


    :\src\KiwiIndexFrmData.cpp(338) : {7517} normal block at 0x00EFFE50, 8 bytes long.
    Data: <! Br > 21 E2 E1 8C 42 72 00 00
    \src\KiwiIndexFrmData.cpp(320) : {7516} normal block at 0x00EF8FA0, 72 bytes long.
    Data: < b DVCRDSRC> 84 62 04 10 CD CD CD CD 44 56 43 52 44 53 52 43
    Object dump complete.
    The thread 0x7C8 has exited with code 0 (0x0).

  2. #2
    Join Date
    Feb 2003
    Location
    Brazil
    Posts
    335

    Re: memory leak

    What kind of data are m_wNumCharaList and m_pCharaList?

    Code:
    class VolRec 
    {
      public:  
      VolRec()
     {
        m_wNumCharaList = 0;
        #ifdef _UNICODE
          m_pCharaList = NULL;
        #else
          m_pCharaList = NULL; 
    
        #endif
    ... 
    
    }
    The compĂ*ler should be complaining about that....

    Rabelo

  3. #3
    Join Date
    Nov 2003
    Posts
    1,405

    Re: memory leak

    You're using an STL vector. Couldn't it be that the standard library manages its own data area and not necessarily gives it back just because a vector is destructed?

  4. #4
    Join Date
    Apr 1999
    Posts
    27,449

    Re: memory leak

    There are things wrong with your code.

    First, please use code tags when posting. Your code is almost impossible to read.

    Second, operator = should return a reference to the current object, not a new object.
    Code:
    VolRec& operator = (const VolRec& vc)
    Third, there is no test for self assignment in your VolRec class. Your code will bomb out if you assign a VolRec to the same VolRec.

    Fourth, you used vector, but why didn't you use it in the VolRec class itself?
    Code:
    #ifdef _UNICODE
     std::vector<WCHAR> m_pCharaList;
    #else
     std::vector<char> m_pCharaList; 
    #endif
    You have the tools (std::vector), you should use them. You now don't need any copy constructor, assignment operator, destructor, or the memory allocation stuff at all when you use a vector instead of a raw char pointers. Here is your entire class done using vector:
    Code:
    #include <vector>
    class VolRec 
    {
       public:
           VolRec()
           {
               m_wNumCharaList = 0;
           }
      
           WORD m_wNumCharaList; 
           #ifdef _UNICODE
           std::vector<WCHAR> m_pCharaList;
           #else
           std::vector<char> m_pCharaList; 
          #endif
    };
    Also, this class is so tiny, unless you are using polymorphism and deriving from the VolRec class, you can now use a std::vector<VolRec> instead of std::vector<VolRec*>.

    The bottom line is that you could write this entire program without using any pointers, only objects. Not only would the code be much smaller, it will more than likely not crash, and would be easily maintainable.

    Regards,

    Paul McKenzie

  5. #5
    Join Date
    Apr 1999
    Posts
    27,449

    Re: memory leak

    Here is a version of the rest of your code that uses no pointers. Note that there also is no need for a destructor or loop to get rid of the elements (note -- this has not been compiled):
    Code:
    class KiwiIndexFrmData : public KiwiIndexFrm
    {
        public:
             KiwiIndexFrmData();
             virtual ~KiwiIndexFrmData();
    
        private:
            std::vector<VolRec> m_vVolRec;
    
        protected:
            bool LoadVolRec(const LPBYTE pb,const DWORD& offset);
    };
    
    
    KiwiIndexFrmData::KiwiIndexFrmData()
    {
    }
    
    
    KiwiIndexFrmData::~KiwiIndexFrmData()
    {
       // not needed
    }
    
    
    bool KiwiIndexFrmData::LoadVolRec(const LPBYTE pb, const DWORD &offset)
    {
       bool bRet = true;
       for(int i = 0;i < ulNumVOLRec;i++ )
       {
            VolRec vc;
            #ifdef _UNICODE
            WORD num = vc.m_wNumCharaList/2; 
            if(num!=0)
            {
               vc.m_pCharaList.resize(num+1);//KiwiIndexFrmData.cpp(338) memory leak
               for(WORD i = 0; i < num; i++)
               {
                  memcpy(&vc.m_pCharaList[i], (void*)(pb+ofVOL+offlocal), sizeof(WCHAR));
                  kpt.WORDInvert_per4bit(vc.m_pCharaList[i]);
                  offlocal = offlocal + sizeof(WCHAR);
               }
               vc.m_pCharaList[num] = '\0';
            }
            #else
            std::vector<WCHAR> pwc(num+1);
            // ...
            //...
            pwc[num] = '\0';
            vc.m_pCharaList.resize(2*num+1);
           ::WideCharToMultiByte(CP_ACP, 0,&pwc[0],num+1,&vc.m_pCharaList[0],2*(num+1),NULL,NULL);
    
            #endif 
    
            m_vVolRec.push_back(vc);
       }
       return bRet;
    }
    Through the usage of using objects, vector, etc. instead of pointers to dynamically allocated memory, the code is much simpler. Note the use of vector::resize() instead of new[whatever].

    Regards,

    Paul McKenzie

  6. #6
    Join Date
    Oct 2000
    Location
    London, England
    Posts
    4,773

    Re: memory leak

    I can answer the original question of where there is a memory leak. In this function:

    Code:
    VolRec operator = (const VolRec& vc)
    {
    ...
      if(m_wNumCharaList!=0)
      {
    #ifdef _UNICODE 
         int num = m_wNumCharaList/2;
         m_pCharaList = new WCHAR[num+1];  
         for(int i = 0; i <= num;i++ )
         {
             m_pCharaList[i] = vc.m_pCharaList[i];
         }
    #else
         int num = m_wNumCharaList;
         m_pCharaList = new char [num+1]; 
         for(int i = 0; i<= num; i++)
         {
             m_pCharaList[i] = vc.m_pCharaList[i];
         }
    #endif
      }
    
    
    ...
    
      return *this;
    }
    You are assigning the pointer with new but it may already have been assigned. And as Paul McKenzie points out, the return type should be VolRec & and not VolRec.

    In addition, it is better for you not to use raw pointers but either std::vector or std::string, with std::wstring for unicode. You can also make your class a template of CHAR_T and use basic_string< CHAR_T > where CHAR_T is either char or or wchar_t

  7. #7
    Join Date
    Nov 2003
    Posts
    24

    Re: memory leak

    thanks all guys replied this thread.
    I have changed my code using the char[256] or WCHAR[256] for resovling...
    I tried using vector<char > or vector <WCHAR> but the character is not right.
    btw.I traced the pragrame using new,but when program exit,the object KiwiIndexfrmdata is deconstructed...pointer in the vecter m_vVolRec and char* m_pcharalist in VolRec is deleted by deconstruction...why the memory leak is still reported by VC6?

  8. #8
    Join Date
    Apr 1999
    Posts
    27,449

    Re: memory leak

    Quote Originally Posted by ephemera
    thanks all guys replied this thread.
    I have changed my code using the char[256] or WCHAR[256] for resovling...
    I tried using vector<char > or vector <WCHAR> but the character is not right.
    Did you get a memory leak using vector? If not, then why not see why the character is not right? That is much easier to fix than a program that is leaking memory.
    btw.I traced the pragrame using new,but when program exit,the object KiwiIndexfrmdata is deconstructed...pointer in the vecter m_vVolRec and char* m_pcharalist in VolRec is deleted by deconstruction...why the memory leak is still reported by VC6?
    We don't have your new program. Your first program had many mistakes, I'm not surprised there is still a memory leak with the new program.

    The bottom line is that once a structure or class contains a pointer to dynamic memory, the structure/class must be safely copyable, assignable, and destructable. To do this, you must code the copy, assignment and destructor correctly so that copying, assignment, and destruction work properly. In all likelihood, these operations are still not coded correctly. For example, the check for self-assignment, the return of an object instead of a reference in the assignment, etc. These must be correct for any class to behave correctly. That's why it was better to use containers such as vector and std::string that automatically copy, assign, and destruct correctly without any extra code.

    Regards,

    Paul McKenzie
    Last edited by Paul McKenzie; September 10th, 2004 at 01:48 AM.

  9. #9
    Join Date
    Nov 2003
    Posts
    1,405

    Re: memory leak

    Quote Originally Posted by ephemera
    ...why the memory leak is still reported by VC6?
    I'm fairly sure the reason is what I suggested in #3. The Standard Library manages its own memory. When a vector is destructed it doesn't necessarily mean that memory is given back to the runtime system. But it's not a leak. The memory will be reused the next time you request a container.

  10. #10
    Join Date
    Nov 2003
    Posts
    24

    Re: memory leak

    thax Paul McKenzie....sorry , my lines dont rightly express my meaning for the sake of my poor english the new program has not memory leak.
    I wanna know why the old one had memory leak?I traced the old one:when deconstructed the KiwiIndexFrmData object,all object created by operator new was deleted,why the memory leak was still reported by VC6?

    Quote Originally Posted by Paul McKenzie
    Did you get a memory leak using vector? If not, then why not see why the character is not right? That is much easier to fix than a program that is leaking memory.
    We don't have your new program. Your first program had many mistakes, I'm not surprised there is still a memory leak with the new program.

    The bottom line is that once a structure or class contains a pointer to dynamic memory, the structure/class must be safely copyable, assignable, and destructable. To do this, you must code the copy, assignment and destructor correctly so that copying, assignment, and destruction work properly. In all likelihood, these operations are still not coded correctly. For example, the check for self-assignment, the return of an object instead of a reference in the assignment, etc. These must be correct for any class to behave correctly. That's why it was better to use containers such as vector and std::string that automatically copy, assign, and destruct correctly without any extra code.

    Regards,

    Paul McKenzie
    Last edited by ephemera; September 10th, 2004 at 10:03 PM.

  11. #11
    Join Date
    Nov 2003
    Posts
    24

    Re: memory leak

    I used code in other place of same program such the same mode as the old code that had memory leak,but that code is ok.but the difference is that the class like VolRec has not copiable function and constuction function, so i am puzzled.
    Quote Originally Posted by _uj
    I'm fairly sure the reason is what I suggested in #3. The Standard Library manages its own memory. When a vector is destructed it doesn't necessarily mean that memory is given back to the runtime system. But it's not a leak. The memory will be reused the next time you request a container.
    Last edited by ephemera; September 10th, 2004 at 10:30 PM.

  12. #12
    Join Date
    Apr 1999
    Posts
    27,449

    Re: memory leak

    Quote Originally Posted by ephemera
    thax Paul McKenzie....sorry , my lines dont rightly express my meaning for the sake of my poor english the new program has not memory leak.
    I wanna know why the old one had memory leak?
    OK. So start with the new program and fix (what seems to be) a simple bug.

    As far as the old program, you need to provide a main() program so that we know what you're doing with your class.
    I traced the old one:when deconstructed the KiwiIndexFrmData object,all object created by operator new was deleted,why the memory leak was still reported by VC6?
    It isn't as simple as tracing a destructor and then that's it. In C++ temporary copies of objects are being done behind the scenes. If you are not aware of this, tracing isn't going to help since you don't have the knowledge required to know what exactly you need to trace. For example, place a cout statement in the constructor and destructor of your class, and see how many times things are being created and destroyed. If the constructions do not match the destructions, then you have proof that your program is leaking memory.

    Regards,

    Paul McKenzie

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •  





Click Here to Expand Forum to Full Width

Featured