Thread: memory leak

  1. #1
    Registered User
    Join Date
    Jan 2010
    Posts
    25

    memory leak

    Hi

    I have an issue with this code, it has memory leak.



    Code:
    #include <windows.h>
    #include <stdio.h>
    #include <string.h>
    #include <dirent.h>
    #include <time.h>
    //#define PORTALPATH "\\\\srv-mantis\\TechDocs\\Portal\\RDP\\password"
    #define PORTALPATH "c:\\temp"
    
    LRESULT CALLBACK WndProc(HWND, UINT, WPARAM, LPARAM);
    
    HINSTANCE g_hinst;
    
    int WINAPI WinMain( HINSTANCE hInstance, HINSTANCE hPrevInstance, 
        LPSTR lpCmdLine, int nCmdShow)
    {
    	HWND hwnd;
    	MSG  msg ;    
    	WNDCLASS wc = {0};
    	wc.lpszClassName = TEXT("Application");
    	wc.hInstance     = hInstance ;
    	wc.hbrBackground = GetSysColorBrush(COLOR_3DFACE);
    	wc.lpfnWndProc   = WndProc ;
    	wc.hCursor       = LoadCursor(0,IDC_ARROW);
    
    	g_hinst = hInstance;
      
    	RegisterClass(&wc);
    	hwnd = CreateWindow(wc.lpszClassName, TEXT("Combo Box"),
    				WS_OVERLAPPEDWINDOW | WS_VISIBLE,
    				300, 300, 270, 170, 0, 0, hInstance, 0);  
    
    
    	while( GetMessage(&msg, NULL, 0, 0)) {
    	DispatchMessage(&msg);
    	}
    	return (int) msg.wParam;
    }
    
    LRESULT CALLBACK WndProc( HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam )
    {
    
    	static HWND hwndCombo, hwndStatic;
    	const TCHAR *items[] = { TEXT("FreeBSD"), TEXT("OpenBSD"), TEXT("Ubuntu"), TEXT("Solaris"), TEXT("1"), TEXT("2"), TEXT("3") };
    	int i;
    	LRESULT sel = 0;
    
    	// get array of files
    
    	typedef char FILENAME[MAX_PATH + 1];
    	// get number of files and then use this size to create the array
    	FILENAME *arrFilename = (FILENAME *)malloc(10000 * sizeof *arrFilename);
        int j,k; 
    	char path[100] = PORTALPATH;
    
    	// directory var
    	DIR *dir;
        struct dirent *ent;
    
    	// set directory of password files 
    	dir = opendir(path);
    	
    	// get filenames
    	if (dir != NULL){
    		
    		j = 0;
    		while((ent = readdir (dir)) != NULL) {
    			//printf("j = %d fname: %s\n",j,ent->d_name) ;
    			//copy string to array
    
    			strcpy(arrFilename[j],ent->d_name);
    			j++;
    		}
    		closedir (dir);
    	} else {
          //could not open directory 
          perror (path);
          //return EXIT_FAILURE;
        }
    	// get array of files END
    
    
    	switch(msg)  
    	{
    		case WM_CREATE:
    			hwndCombo = CreateWindow(TEXT("combobox"), NULL, 
    					WS_CHILD | WS_VISIBLE | CBS_DROPDOWN | WS_VSCROLL,
    					10, 10, 120, 510, hwnd, NULL, g_hinst, NULL);   
    
    			CreateWindow(TEXT("button"), TEXT("Drop down"), 
    					WS_CHILD | WS_VISIBLE,
    					150, 10, 90, 25, hwnd, (HMENU)1, g_hinst, NULL); 
    
    			hwndStatic = CreateWindow(TEXT("static"), TEXT(""), 
    					WS_CHILD | WS_VISIBLE,
    					150, 80, 90, 25, hwnd, NULL, g_hinst, NULL); 
    			
    			// enter filenames into combobox
    			
    			for ( i = 0; i < 7; i++ ) {
    				SendMessage(hwndCombo, CB_ADDSTRING, 0, (LPARAM) items[i]);
    			}
    			
    			break;
    
    		case WM_COMMAND:
    			if (HIWORD(wParam) == BN_CLICKED) {
    				SendMessage(hwndCombo, CB_SHOWDROPDOWN, (WPARAM) TRUE, 0);
    			}
                 
    			if ( HIWORD(wParam) == CBN_SELCHANGE) {               
    				sel = SendMessage(hwndCombo, CB_GETCURSEL, 0, 0);
    				SetWindowText(hwndStatic, items[sel]);
    				SetFocus(hwnd);
    			}
    
    			break;
    
    		case WM_DESTROY:
    			PostQuitMessage(0);
    			break; 
    	}
    	return DefWindowProc(hwnd, msg, wParam, lParam);
    }

  2. #2
    - - - - - - - - oogabooga's Avatar
    Join Date
    Jan 2008
    Posts
    2,808
    Your WndProc is badly designed.

    Firstly, you should not be calling DefWindowProc if you have handled the message yourself. It's only for messages that you do not handle.

    Secondly, and here's your real problem, WndProc is called many, many, many times, and each time it's called you read the files again. You never free this memory, so it eventually crashes.

    I can't tell you specifically how to fix it since your code is clearly not finished (you don't use the filenames anywhere).

    EDIT:
    Now that I think about it, you should move the whole directory reading part into the WM_CREATE handler and make arrFilename static. Free the memory in WM_DESTROY.
    Last edited by oogabooga; 09-17-2012 at 04:12 PM.
    The cost of software maintenance increases with the square of the programmer's creativity. - Robert D. Bliss

  3. #3
    Registered User
    Join Date
    Jan 2010
    Posts
    25
    yes I am not using it because I am getting Chinese chars in drop down box if I put it here

    SendMessage(hwndCombo, CB_ADDSTRING, 0, (LPARAM) items[i]);

    changed:
    SendMessage(hwndCombo, CB_ADDSTRING, 0, (LPARAM) arrFilename[i]);

    can you advise how to use it here?

  4. #4
    - - - - - - - - oogabooga's Avatar
    Join Date
    Jan 2008
    Posts
    2,808
    You're probably using unicode, but the readdir function doesn't.

    One way to solve this is to turn off unicode:
    * right-click on the name of your project in the "solution explorer" (at the left)
    * choose "properties"
    * under "general" set "character set" to "not set".
    * recompile
    The cost of software maintenance increases with the square of the programmer's creativity. - Robert D. Bliss

  5. #5
    Registered User
    Join Date
    Jan 2010
    Posts
    25
    Quote Originally Posted by oogabooga View Post
    You're probably using unicode, but the readdir function doesn't.

    One way to solve this is to turn off unicode:
    * right-click on the name of your project in the "solution explorer" (at the left)
    * choose "properties"
    * under "general" set "character set" to "not set".
    * recompile
    Yes I know that it is the unicode issue, but if I will need to use it later on, how can I fix it now?

  6. #6
    - - - - - - - - oogabooga's Avatar
    Join Date
    Jan 2008
    Posts
    2,808
    Did you try turning unicode off? What happened?

    To continue using unicode you have two options.
    1. continue using readdir and convert your strings
    2. use the Windows-specific functions FindFirstFile, FindNextFile

    Here's an example of options 2:
    Listing the Files in a Directory (Windows)
    The cost of software maintenance increases with the square of the programmer's creativity. - Robert D. Bliss

  7. #7
    Registered User
    Join Date
    Jan 2010
    Posts
    25
    i would prefer to do option 1 for now, can you advise how to convert the strings?
    1. continue using readdir and convert your strings

  8. #8
    Registered User
    Join Date
    Jan 2010
    Posts
    25
    Quote Originally Posted by oogabooga View Post
    Your WndProc is badly designed.

    Firstly, you should not be calling DefWindowProc if you have handled the message yourself. It's only for messages that you do not handle.
    ok you say that I shouldn't use but I see it in Petzold book example

    Code:
    HELLOWIN.C
    
    /*------------------------------------------------------------
       HELLOWIN.C -- Displays "Hello, Windows 98!" in client area
                     (c) Charles Petzold, 1998
      ------------------------------------------------------------*/
    
    #include <windows.h>
    
    LRESULT CALLBACK WndProc (HWND, UINT, WPARAM, LPARAM) ;
    
    int WINAPI WinMain (HINSTANCE hInstance, HINSTANCE hPrevInstance,
                        PSTR szCmdLine, int iCmdShow)
    {
         static TCHAR szAppName[] = TEXT ("HelloWin") ;
         HWND         hwnd ;
         MSG          msg ;
         WNDCLASS     wndclass ;
    
         wndclass.style         = CS_HREDRAW | CS_VREDRAW ;
         wndclass.lpfnWndProc   = WndProc ;
         wndclass.cbClsExtra    = 0 ;
         wndclass.cbWndExtra    = 0 ;
         wndclass.hInstance     = hInstance ;
         wndclass.hIcon         = LoadIcon (NULL, IDI_APPLICATION) ;
         wndclass.hCursor       = LoadCursor (NULL, IDC_ARROW) ;
         wndclass.hbrBackground = (HBRUSH) GetStockObject (WHITE_BRUSH) ;
         wndclass.lpszMenuName  = NULL ;
         wndclass.lpszClassName = szAppName ;
    
         if (!RegisterClass (&wndclass))
         {
              MessageBox (NULL, TEXT ("This program requires Windows NT!"), 
                          szAppName, MB_ICONERROR) ;
              return 0 ;
         }
         hwnd = CreateWindow (szAppName,                  // window class name
                              TEXT ("The Hello Program"), // window caption
                              WS_OVERLAPPEDWINDOW,        // window style
                              CW_USEDEFAULT,              // initial x position
                              CW_USEDEFAULT,              // initial y position
                              CW_USEDEFAULT,              // initial x size
                              CW_USEDEFAULT,              // initial y size
                              NULL,                       // parent window handle
                              NULL,                       // window menu handle
                              hInstance,                  // program instance handle
                              NULL) ;                     // creation parameters
         
         ShowWindow (hwnd, iCmdShow) ;
         UpdateWindow (hwnd) ;
         
         while (GetMessage (&msg, NULL, 0, 0))
         {
              TranslateMessage (&msg) ;
              DispatchMessage (&msg) ;
         }
         return msg.wParam ;
    }
    
    LRESULT CALLBACK WndProc (HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam)
    {
         HDC         hdc ;
         PAINTSTRUCT ps ;
         RECT        rect ;
         
         switch (message)
         {
         case WM_CREATE:
              PlaySound (TEXT ("hellowin.wav"), NULL, SND_FILENAME | SND_ASYNC) ;
              return 0 ;
    
         case WM_PAINT:
              hdc = BeginPaint (hwnd, &ps) ;
              
              GetClientRect (hwnd, &rect) ;
              
              DrawText (hdc, TEXT ("Hello, Windows 98!"), -1, &rect,
                        DT_SINGLELINE | DT_CENTER | DT_VCENTER) ;
              EndPaint (hwnd, &ps) ;
              return 0 ;
              
         case WM_DESTROY:
              PostQuitMessage (0) ;
              return 0 ;
         }
         return DefWindowProc (hwnd, message, wParam, lParam) ;
    }

  9. #9
    - - - - - - - - oogabooga's Avatar
    Join Date
    Jan 2008
    Posts
    2,808
    Quote Originally Posted by *nick View Post
    ok you say that I shouldn't use but I see it in Petzold book example
    I didn't say that you shouldn't use it; you have to use it. I said that you shouldn't call it if you've handled the message yourself. Notice that Petzold's example returns if it handles a message and only calles DefWindowProc if it doesn't handle the message. Basically you just have to change your breaks to return 0.
    The cost of software maintenance increases with the square of the programmer's creativity. - Robert D. Bliss

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Memory leak
    By ssharish2005 in forum General Discussions
    Replies: 35
    Last Post: 07-05-2011, 11:10 PM
  2. Possible memory leak.
    By msh in forum C++ Programming
    Replies: 11
    Last Post: 07-21-2009, 12:28 PM
  3. Replies: 2
    Last Post: 09-28-2006, 01:06 PM
  4. Memory leak or no?
    By rafe in forum C++ Programming
    Replies: 4
    Last Post: 10-02-2002, 04:33 PM
  5. Memory leak?
    By Weed4Me in forum C Programming
    Replies: 2
    Last Post: 11-09-2001, 05:47 PM