[Okta Webinar] Learn how to a build a cloud-first strategyRegister Now

x
  • Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 367
  • Last Modified:

memory leak problems? refreshing tree control.

expanding ojn the question i asked here

http://www.experts-exchange.com/Programming/Languages/CPP/Q_22719299.html

i think i'm not cleaning my memory up properly or something similar as my program seems to be running out of memory. or i get user break points.

basically i have a program which the user uses to create a structure and displays this in a CTreeCtrl. this is all working fine however, as i want the user to be able to edit this structure, ie. add\remove nodes, i have put a refresh method call in paint method for the dialog.

basically a user will click a button to add a node, the refresh is then called here (as well as in the paint method as it seems to not want to work if i don't). i can add two or three no problem however after i add three or more eventually i get a user break point , or out of memory error and the program crashes out.

can anyone tell me what could be going on. in the refresh method i basically remove all the items from the tree and read the information in the structures i have created.

BUTTON* rBut;
HOOK* rHook;
char var[80];
char tmp[10];
TV_INSERTSTRUCT g_tree;
HTREEITEM Parent;
ofstream logfile;
void CAddHookDlg::refreshHookFile()
{
      m_tree_button.DeleteAllItems(); //remove for refresh
      
      //now insert all hooks
      int i;
      for(i=0; i<maxHooks; i++)
      {
            rHook = getHook(i);

            if(rHook == NULL)
                  break;

            logfile.open( "refreshlog.txt", ios::app );
            logfile << "REFRESH:" << rHook->window << ": \n";
            logfile.close();

            g_tree.item.iImage=0; // not pressed pic
            g_tree.item.iSelectedImage=1; // pressed pic
            g_tree.hParent = NULL;
            g_tree.hInsertAfter = TVI_LAST;
            g_tree.item.mask=TVIF_TEXT|TVIF_IMAGE|TVIF_SELECTEDIMAGE;

            strcpy(var, "HOOK:");
            strcat(var, rHook->window);
            g_tree.item.pszText= var;

            Parent=m_tree_button.InsertItem(&g_tree);

            //now add each button for this particular hook
            int k;
            for(k=0; k<maxButtons; k++)
            {
                  rBut = getButton(i,k);

                  if(rBut == NULL)
                        break;
                  
                  g_tree.item.iImage=0; // not pressed pic
                  g_tree.item.iSelectedImage=1; // pressed pic
                  g_tree.hParent = Parent;
                  g_tree.hInsertAfter = TVI_LAST;
                  g_tree.item.mask=TVIF_TEXT|TVIF_IMAGE|TVIF_SELECTEDIMAGE;

                  strcpy(var, "BUTTON:");
                  strcat(var, rBut->text);
                  g_tree.item.pszText= var;

                  Parent=m_tree_button.InsertItem(&g_tree);
                  g_tree.item.iImage=2; // not pressed pic
                  g_tree.item.iSelectedImage=2; // pressed pic
                              
                  strcpy(var, "CLASS:");

                  strcat(var, rBut->bClass); //got to here?
                  g_tree.hParent = Parent;
                  g_tree.item.pszText= var;
                  m_tree_button.InsertItem(&g_tree);

                  char x[10],y[10];
                  itoa(rBut->dimensions->x,x,10);
                  itoa(rBut->dimensions->y,y,10);

                  strcpy(var,"COORDS:[x");
                  strcat(var, x);
                  strcat(var,",y");
                  strcat(var, y);
                  strcat(var,"]\0");

                  g_tree.hParent = Parent;
                  g_tree.item.pszText= var;
                  m_tree_button.InsertItem(&g_tree);

                  strcpy(var, "WIDTH:");
                  itoa(rBut->dimensions->width,tmp,10);
                  strcat(var,tmp);

                  g_tree.item.pszText= var;
                  m_tree_button.InsertItem(&g_tree);

                  strcpy(var, "HEIGHT:");
                  itoa(rBut->dimensions->height,tmp,10);
                  strcat(var,tmp);

                  g_tree.item.pszText= var;
                  m_tree_button.InsertItem(&g_tree);

                  strcpy(var, "ANCHOR:");      
                  int anchor = rBut->dimensions->anchor;

                  switch(anchor)
                  {
                  case 0:
                        strcat(var,"T_LEFT");
                        break;
                  case 1:
                        strcat(var,"T_RIGHT");
                        break;
                  case 2:
                        strcat(var,"B_LEFT");
                        break;
                  case 3:
                        strcat(var,"B_RIGHT");
                        break;
                  }

                  g_tree.item.pszText= var;
                  m_tree_button.InsertItem(&g_tree);

                  strcpy(var, "PROGRAM:");
                  strcat(var, getButtonProgram(i,k));

                  g_tree.item.pszText= var;
                  m_tree_button.InsertItem(&g_tree);

                  strcpy(var, "OUTPUT:");
                  strcat(var, getButtonOutput(i,k));

                  g_tree.item.pszText= var;
                  m_tree_button.InsertItem(&g_tree);

                  //now add fields on button
                  g_tree.item.iImage=0; // not pressed pic
                  g_tree.item.iSelectedImage=1; // pressed pic
                  g_tree.item.pszText= "FIELDS:";
                  Parent = m_tree_button.InsertItem(&g_tree);

                  g_tree.item.iImage=2; // not pressed pic
                  g_tree.item.iSelectedImage=2; // pressed pic
                  g_tree.hParent = Parent;

                  int l;
                  for(l=0; l<maxFields; l++)
                  {
                        if(getButtonField(i,k,l)==NULL)
                              break;

                        itoa(getButtonField(i,k,l),tmp,10);
                        g_tree.item.pszText= tmp;
                  }
            }
      }
}


0
flynny
Asked:
flynny
  • 5
  • 5
1 Solution
 
ravenplCommented:
What 'getHook(i);' does, allocates new object? If so, then old one is not freed.
Very same applies to getButton(i,k);

Either check for nonNULL before those two calls, and call delete(rHook) before assigning new value, or use
std::auto_ptr

> BUTTON* rBut;
> HOOK* rHook;
#include <memory>
std::auto_ptr<BUTTON> rBut;
std::auto_ptr<HOOK> rHook;

...
rHook.reset( getButton(i,k) ); //delete old(if any) object, keep new one
rHook.reset(); //delete object
rHook.release(); //don't delete the object, just forget the pointer, user have to delete it by itself
also, aut_ptr deletes the object it holds if the auto_ptr object is deleted. In such case it behaves like scoped ptr.
0
 
ravenplCommented:
But note: the above is valid only if mentioned functions create new objects. If they return object kept in some structures, objects should not be deleted.

Also
getButtonProgram(i,k); getButtonOutput(i,k); return char*, but if it's newly allocated memory, then is never freed

when You strcat(), You sure You will not run outside the buffer? Maybe You should use c++ string class - it would reallocate itself automatically.
0
 
flynnyAuthor Commented:
hi thanks for the reply,

the methods simply return an object held in the structure

HOOK* getHook(int i){ return hookFile.hooks[i]; }
BUTTON* getButton(int i, int j){ return hookFile.hooks[i]->buttons[j]; }

ok, i've replaced it with CString too and made a few changes however it still does the same if i add four item and click on a selection and randomly highlight differnet ones after a few click on of the entries becomes ||||||| and then when i close the dialog of or click another button i get the user break point.
heres the new method.

BUTTON* rBut;
HOOK* rHook;
char* var;
CString cVar;
CString x,y;
char tmp[10]; //needed for no conversion
TV_INSERTSTRUCT g_tree; //for listing the hook file
HTREEITEM nHook;
HTREEITEM nButton;
HTREEITEM nField;

ofstream logfile;

void CAddHookDlg::refreshHookFile()
{
      m_tree_button.DeleteAllItems(); //remove for refresh
      
      //now insert all hooks
      int i;
      for(i=0; i<maxHooks; i++)
      {
            rHook = getHook(i);

            if(rHook == NULL)
                  break;

            logfile.open( "refreshlog.txt", ios::app );
            logfile << "REFRESH:" << rHook->window << ": \n";
            logfile.close();

            g_tree.item.iImage=0; // not pressed pic
            g_tree.item.iSelectedImage=1; // pressed pic
            g_tree.hParent = NULL;
            g_tree.hInsertAfter = TVI_LAST;
            g_tree.item.mask=TVIF_TEXT|TVIF_IMAGE|TVIF_SELECTEDIMAGE;

            cVar = "HOOK:";
            cVar = cVar + rHook->window;
            var = (char*) malloc(strlen(cVar)+1);
            strcpy(var,cVar);
            g_tree.item.pszText = var;
            nHook=m_tree_button.InsertItem(&g_tree);
            free(var);
            
            //now add each button for this particular hook
            int k;
            for(k=0; k<maxButtons; k++)
            {
                  rBut = rHook->buttons[k];

                  if(rBut == NULL)
                        break;
                  
                  g_tree.item.iImage=0; // not pressed pic
                  g_tree.item.iSelectedImage=1; // pressed pic
                  g_tree.hParent = nHook;
                  g_tree.hInsertAfter = TVI_LAST;
                  g_tree.item.mask=TVIF_TEXT|TVIF_IMAGE|TVIF_SELECTEDIMAGE;

                  cVar = "BUTTON:";
                  cVar = cVar + rBut->text;
                  var = (char*) malloc(strlen(cVar)+1);
                  strcpy(var,cVar);
                  g_tree.item.pszText= var;
                  nButton=m_tree_button.InsertItem(&g_tree);
                  free(var);


                  g_tree.item.iImage=2; // not pressed pic
                  g_tree.item.iSelectedImage=2; // pressed pic
                  g_tree.hParent = nButton;
                              
                  cVar = "CLASS:";
                  cVar = cVar + rBut->bClass;
                  var = (char*) malloc(strlen(cVar)+1);
                  strcpy(var,cVar);
                  g_tree.item.pszText= var;
                  m_tree_button.InsertItem(&g_tree);
                  free(var);

                  x.Format("%d",rBut->dimensions->x);
                  y.Format("%d",rBut->dimensions->y);
                  cVar = "COORDS:[x" + x + ",y" + y + "]";
                  var = (char*) malloc(strlen(cVar)+1);
                  strcpy(var,cVar);
                  g_tree.item.pszText= var;
                  m_tree_button.InsertItem(&g_tree);
                  free(var);

                  cVar = "WIDTH:" + rBut->dimensions->width;
                  var = (char*) malloc(strlen(cVar)+1);
                  strcpy(var,cVar);
                  g_tree.item.pszText= var;
                  m_tree_button.InsertItem(&g_tree);
                  free(var);

                  cVar = "HEIGHT:" + rBut->dimensions->height;
                  var = (char*) malloc(strlen(cVar)+1);
                  strcpy(var,cVar);
                  g_tree.item.pszText= var;
                  m_tree_button.InsertItem(&g_tree);
                  free(var);

                  cVar = "ANCHOR:";      
                  int anchor = rBut->dimensions->anchor;

                  switch(anchor)
                  {
                  case 0:
                        cVar = cVar + "T_LEFT";
                        break;
                  case 1:
                        cVar = cVar + "T_RIGHT";
                        break;
                  case 2:
                        cVar = cVar + "B_LEFT";
                        break;
                  case 3:
                        cVar = cVar + "B_RIGHT";
                        break;
                  }

                  var = (char*) malloc(strlen(cVar)+1);
                  strcpy(var,cVar);
                  g_tree.item.pszText= var;
                  m_tree_button.InsertItem(&g_tree);
                  free(var);


                  cVar = "PROGRAM:";
                  cVar = cVar + rBut->program;
                  var = (char*) malloc(strlen(cVar)+1);
                  strcpy(var,cVar);
                  g_tree.item.pszText= var;
                  m_tree_button.InsertItem(&g_tree);
                  free(var);

                  cVar = "OUTPUT:";
                  cVar = cVar + rBut->output;
                  var = (char*) malloc(strlen(cVar)+1);
                  strcpy(var,cVar);
                  g_tree.item.pszText= var;
                  m_tree_button.InsertItem(&g_tree);
                  free(var);

                  //now add fields on button
                  g_tree.item.iImage=0; // not pressed pic
                  g_tree.item.iSelectedImage=1; // pressed pic
                  g_tree.item.pszText= "FIELDS:";
                  nField = m_tree_button.InsertItem(&g_tree);

                  g_tree.item.iImage=2; // not pressed pic
                  g_tree.item.iSelectedImage=2; // pressed pic
                  g_tree.hParent = nField;

                  int l;
                  for(l=0; l<maxFields; l++)
                  {
                        if(rBut->fields[l]==NULL)
                              break;

                        cVar.Format("%d",rBut->fields[l]->fieldPath);
                        var = (char*) malloc(strlen(cVar)+1);
                        strcpy(var,cVar);
                        g_tree.item.pszText= var;
                        m_tree_button.InsertItem(&g_tree);
                        free(var);
                  }
            }
      }
}
0
What does it mean to be "Always On"?

Is your cloud always on? With an Always On cloud you won't have to worry about downtime for maintenance or software application code updates, ensuring that your bottom line isn't affected.

 
ravenplCommented:
OK, I don't know where the leak can happen. Maybe You should try some memory debuggers like valgrind or redefine
new delete malloc free and debugLog each operation.

Now, I see next thing. The following (or similar) repeats
                  cVar = "OUTPUT:";
                  cVar = cVar + rBut->output;
                  var = (char*) malloc(strlen(cVar)+1);
                  strcpy(var,cVar);
                  g_tree.item.pszText= var; //<----------------- HERE
                  m_tree_button.InsertItem(&g_tree);
                  free(var);//<--------------- HERE

if g_tree.item.pszText is "char*" and not string, then it references to freed memory.

if g_tree.item.pszText is string, then why not do the following
                  cVar = "OUTPUT:";
                  cVar.append(rBut->output);
                  g_tree.item.pszText= var;
                  m_tree_button.InsertItem(&g_tree);
0
 
flynnyAuthor Commented:
i see this was what i tried at first however, the reason i've been copying the cVar into a char* var is because the compiler complained about cVar nt been a char*. this was the only way i could think of for casting cVar to a char* from a CString.
0
 
ravenplCommented:
cVar.c_str() extracts const char * from the cVar. But it's not copy, it's reference to cVar internal structures.
0
 
flynnyAuthor Commented:
hi ive tried

cVar = "BUTTON:";
cVar = cVar + rBut->text;
g_tree.item.pszText= cVar.c_str();
nButton=m_tree_hook.InsertItem(&g_tree);
free(var);

and it says c_str() is not a member of CString. also it says .append is not either when i try that.
0
 
ravenplCommented:
Right, somehow I was thinking of std::string
http://msdn2.microsoft.com/en-us/library/awkwbzyc(VS.71).aspx#_core_converting_to_c.2d.style_null.2d.terminated_strings
either copy or cast.
In both cases if
g_tree.item.pszText;
is char *, then it will dereference already freed memory;
0
 
flynnyAuthor Commented:
ok. thanks i've also found another problem that is related (i think ) to memory problems. In my program i have an open button which will read in a file and fill in the structure.

If i run this the first time then it works with no problems, however if i then try and rea din the same file that it read in successfully before i get a user break point exception again.

This seems to happen at different points in the program. I have noticed that every now and again it causes an access violation on this method

void addButton(CString hook, CString text, CString bClass, int x, int y, int width, int height, int anchor, CString program, CString output, int* fields)
{
      int i;
      for(i=0; i<maxHooks; i++)
            if(strcmp(hookFile.hooks[i]->window,hook) ==0)
                  break;

      int j;
      for(j=0; j<maxButtons; j++)
            if(hookFile.hooks[i]->buttons[j] == NULL)
                  break;

      if(i<maxHooks && i<maxButtons)
      {
            BUTTON *pBut = (BUTTON*) GlobalAlloc(GPTR, sizeof(BUTTON));

            HGLOBAL memPtr = GlobalAlloc(GMEM_FIXED, strlen(bClass)+1); //pointer to mem block

            if( memPtr && (pBut->bClass = (LPTSTR)GlobalLock(memPtr)) != NULL )
                  RtlMoveMemory(pBut->bClass, bClass, strlen(bClass)+1);
            else
            {
                  char buf[128];
                  sprintf(buf, "Error Unable to allocate Memory for button class.");
                  MessageBox(NULL, buf, NULL, 0);
            }

            memPtr = GlobalAlloc(GMEM_FIXED, strlen(text)+1); //pointer to mem block

            if( memPtr && (pBut->text = (LPTSTR)GlobalLock(memPtr)) != NULL )
                  RtlMoveMemory(pBut->text, text, strlen(text)+1);
            else
            {
                  char buf[128];
                  sprintf(buf, "Error Unable to allocate Memory for button text.");
                  MessageBox(NULL, buf, NULL, 0);
            }

            DIMENSIONS *pDim = (DIMENSIONS*) GlobalAlloc(GPTR, sizeof(DIMENSIONS));

            memPtr = GlobalAlloc(GMEM_FIXED, sizeof(int)); //pointer to mem block

            if( memPtr && (pDim->x = (int)GlobalLock(memPtr)) != NULL )
                  pDim->x = x;
            else
            {
                  char buf[128];
                  sprintf(buf, "Error Unable to allocate Memory for button x coord.");
                  MessageBox(NULL, buf, NULL, 0);
            }

            memPtr = GlobalAlloc(GMEM_FIXED, sizeof(int)); //pointer to mem block

            if( memPtr && (pDim->y = (int)GlobalLock(memPtr)) != NULL )
                  pDim->y = y;
            else
            {
                  char buf[128];
                  sprintf(buf, "Error Unable to allocate Memory for button y coord.");
                  MessageBox(NULL, buf, NULL, 0);
            }

            memPtr = GlobalAlloc(GMEM_FIXED, sizeof(int)); //pointer to mem block

            if( memPtr && (pDim->height = (int)GlobalLock(memPtr)) != NULL )
                  pDim->height = height;
            else
            {
                  char buf[128];
                  sprintf(buf, "Error Unable to allocate Memory for button height value.");
                  MessageBox(NULL, buf, NULL, 0);
            }

            memPtr = GlobalAlloc(GMEM_FIXED, sizeof(int)); //pointer to mem block

            if( memPtr && (pDim->width = (int)GlobalLock(memPtr)) != NULL )
                  pDim->width = width;
            else
            {
                  char buf[128];
                  sprintf(buf, "Error Unable to allocate Memory for button width value.");
                  MessageBox(NULL, buf, NULL, 0);
            }

            memPtr = GlobalAlloc(GMEM_FIXED, sizeof(int)); //pointer to mem block

            if( memPtr && (pDim->anchor = (int)GlobalLock(memPtr)) != NULL)
                  pDim->anchor = anchor;
            else
            {
                  char buf[128];
                  sprintf(buf, "Error Unable to allocate Memory for button anchor.");
                  MessageBox(NULL, buf, NULL, 0);
            }

            pBut->dimensions = pDim;

            memPtr = GlobalAlloc(GMEM_FIXED, strlen(program)+1); //pointer to mem block

            if( memPtr && (pBut->program = (LPTSTR)GlobalLock(memPtr)) != NULL )
                  RtlMoveMemory(pBut->program, program, strlen(program)+1);
            else
            {
                  char buf[128];
                  sprintf(buf, "Error Unable to allocate Memory for button program.");
                  MessageBox(NULL, buf, NULL, 0);
            }

            memPtr = GlobalAlloc(GMEM_FIXED, strlen(output)+1); //pointer to mem block

            if( memPtr && (pBut->output = (LPTSTR)GlobalLock(memPtr)) != NULL )
                  RtlMoveMemory(pBut->output, output, strlen(output)+1);
            else
            {
                  char buf[128];
                  sprintf(buf, "Error Unable to allocate Memory for button output.");
                  MessageBox(NULL, buf, NULL, 0);
            }

            //now add fields
            addField(pBut,fields);

            hookFile.hooks[i]->buttons[j] = pBut;

      }
      else
      {
            char buf[128];
            sprintf(buf, "Unable to add button maxHooks=%d, maxButtons=%d", maxHooks, maxButtons);
            MessageBox(NULL, buf, NULL, 0);
      }
}

do you think this method which populates the button structure could be the problem? i've also raised the points to 500 ;) thanks for your help.
0
 
flynnyAuthor Commented:
hi from using break points i think the problem is in the addButton method as it runs through this fine first time then on the second run through in my readfile method it breaks on calling this method
0

Featured Post

New feature and membership benefit!

New feature! Upgrade and increase expert visibility of your issues with Priority Questions.

  • 5
  • 5
Tackle projects and never again get stuck behind a technical roadblock.
Join Now