Memory leak issues! Please help! >_<

This is a discussion on Memory leak issues! Please help! >_< within the C Programming forums, part of the General Programming Boards category; For some reason I'm getting a memory leak error. It's probably from the part where I start to copy the ...

  1. #1
    Registered User
    Join Date
    Nov 2008
    Posts
    21

    Memory leak issues! Please help! >_<

    For some reason I'm getting a memory leak error. It's probably from the part where I start to copy the strings and allocate an array of characters, but I have no idea why. I'm using a text file for input data, which is found here: http://puma.deanza.edu/distribute/De.../countries.txt

    I appreciate anyone who helps me solve this issue :]

    Code:
    #include <stdio.h>
    #include <stdlib.h>
    #include <crtdbg.h>
    #define FILE_IN "countries.txt" // user is free to change filename
    
    // Prototype Declarations
    char **getNames ( void );
    
    int main (void)
    {
    //  Local Definitions 
    	char **countryList;
    	int i;
    
    //  Statements 
    	printf("\t\t LAB 6 - Strings\n\n" );
    	countryList = getNames();
    	//for (i=0;i<=13;i++)
    		//free(countryList[i]);
    	free(countryList);
    
        printf( _CrtDumpMemoryLeaks() ? "Memory Leak\n" : "No Leak\n");
    	printf("\n\t\tEnd of Lab 6\n"
    		   "\n\t\tHave a great day!\n");
       return 0;
    
    } // main
    
    char **getNames ( void )
    {
    // Local Declarations
    	FILE *spIn;
    	int numCountries = 0;
    	char tempAry[50];
    	char **countryList;
    	int i;
    	int closeResult;
    
    // Statements
    	if ( ! ( spIn = fopen ( FILE_IN , "r" ) ) )
    	{
    		printf("Error loading input file.\n");
    		exit(100);
    	}
    	while(fgets(tempAry, sizeof(tempAry), spIn))
    		numCountries++;
    	printf("Number of countries: %d\n", numCountries);
    	closeResult = fclose ( spIn );
    	if (closeResult==EOF)
    	{
    		printf("File did not close properly.\n");
    		exit(101);
    	}
    	if ( ! ( spIn = fopen ( FILE_IN , "r" ) ) )
    	{
    		printf("Error loading input file.\n");
    		exit(102);
    	}
    	if ( ! ( countryList = (char**) calloc (numCountries + 1, sizeof(char*))))
    	{
    		printf("Memory is unavailable.\n");
    		exit(103);
    	}
    	for ( i = 0; i < numCountries; i++ )
    	{
    		fgets(tempAry, sizeof(tempAry), spIn);
    		countryList[i] = (char*) calloc (strlen(tempAry), sizeof(char));
    		strcpy(countryList[i], tempAry);
    		printf("%s", countryList[i]);
    	}
    	countryList[i] = (char*) calloc (strlen(tempAry), sizeof(char));
    	countryList[numCountries] = '\0';
    
    	closeResult = fclose ( spIn );
    	if (closeResult==EOF)
    	{
    		printf("File did not close properly.\n");
    		exit(104);
    	}
    
       return countryList;
    } // getNames

  2. #2
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,185
    Because you commented out the part where you returned all the memory?

  3. #3
    Registered User
    Join Date
    Nov 2008
    Posts
    21
    Quote Originally Posted by tabstop View Post
    Because you commented out the part where you returned all the memory?
    My compiler gives me a heap stack error when I run that.

  4. #4
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,185
    Do you have 14 countries? In fact, when you get back to main, how do you know how many countries you have? (Answer: You have an empty country, so loop until you get to that blank country, not until you have 14 countries.)

  5. #5
    Registered User
    Join Date
    Nov 2008
    Posts
    21
    The last country is a null pointer. Our teacher wants us to put a null pointer at the end of any array of pointers. I just put the numbers there assuming that the parameters would pass the value of the number of countries just to speed things up.

  6. #6
    Algorithm Dissector iMalc's Avatar
    Join Date
    Dec 2005
    Location
    New Zealand
    Posts
    6,308
    Code:
    		countryList[i] = (char*) calloc (strlen(tempAry), sizeof(char));
    You need +1 for the NULL there.
    My homepage
    Advice: Take only as directed - If symptoms persist, please see your debugger

    Linus Torvalds: "But it clearly is the only right way. The fact that everybody else does it some other way only means that they are wrong"

  7. #7
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,185
    Quote Originally Posted by RaDeuX View Post
    I just put the numbers there assuming that the parameters would pass the value of the number of countries just to speed things up.
    That may be the silliest thing I've ever seen. You expected the compiler to replace the literal value 13 with the value of i from another function?

  8. #8
    Registered User
    Join Date
    Nov 2008
    Posts
    21
    Quote Originally Posted by tabstop View Post
    That may be the silliest thing I've ever seen. You expected the compiler to replace the literal value 13 with the value of i from another function?
    That's not the point. The 13 is temporary. What I'm trying to fix is the memory leak, not the functionality of the program.

  9. #9
    Registered User
    Join Date
    Nov 2008
    Posts
    21
    Quote Originally Posted by iMalc View Post
    Code:
    		countryList[i] = (char*) calloc (strlen(tempAry), sizeof(char));
    You need +1 for the NULL there.
    No I don't. i is 13 after the for loop ends.

  10. #10
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,185
    Quote Originally Posted by RaDeuX View Post
    That's not the point. The 13 is temporary. What I'm trying to fix is the memory leak, not the functionality of the program.
    Then free all your memory.
    Code:
    i = 0;
    while (countryList[i][0] != '\0') {
        free(countryList[i]);
        i++;
    }
    free(countryList[i]); /* the one at the end that is the \0 character */
    free(countryList); /* now you can do such a thing */

  11. #11
    Registered User
    Join Date
    Nov 2008
    Posts
    21
    Quote Originally Posted by tabstop View Post
    Then free all your memory.
    Code:
    i = 0;
    while (countryList[i][0] != '\0') {
        free(countryList[i]);
        i++;
    }
    free(countryList[i]); /* the one at the end that is the \0 character */
    free(countryList); /* now you can do such a thing */
    That causes the same debug error. It tells me that its detected a heap corruption.

  12. #12
    Registered User
    Join Date
    Nov 2008
    Posts
    21
    Here's the updated version:
    Code:
    #include <stdio.h>
    #include <stdlib.h>
    #include <crtdbg.h>
    #define FILE_IN "countries.txt" // user is free to change filename
    
    // Prototype Declarations
    char **getNames ( int *numCountries );
    
    int main (void)
    {
    //  Local Definitions
    	char **countryList;
    	int numCountries = 0;
    	int i;
    
    //  Statements 
    	printf("\t\t LAB 6 - Strings\n\n" );
    	countryList = getNames(&numCountries);
    	//for (i=0;i<=numCountries;i++)
    		//free(countryList[i]);
    	
    	free(countryList);
    	
        printf( _CrtDumpMemoryLeaks() ? "Memory Leak\n" : "No Leak\n");
    	printf("\n\t\tEnd of Lab 6\n"
    		   "\n\t\tHave a great day!\n");
       return 0;
    
    } // main
    
    char **getNames ( int *numCountries )
    {
    // Local Declarations
    	FILE *spIn;
    	char tempAry[50];
    	char **countryList;
    	int i;
    	int closeResult;
    
    // Statements
    	if ( ! ( spIn = fopen ( FILE_IN , "r" ) ) )
    	{
    		printf("Error loading input file.\n");
    		exit(100);
    	}
    	while(fgets(tempAry, sizeof(tempAry), spIn))
    		(*numCountries)++;
    	printf("Number of countries: &#37;d\n", *numCountries);
    	closeResult = fclose ( spIn );
    	if (closeResult==EOF)
    	{
    		printf("File did not close properly.\n");
    		exit(101);
    	}
    	if ( ! ( spIn = fopen ( FILE_IN , "r" ) ) )
    	{
    		printf("Error loading input file.\n");
    		exit(102);
    	}
    	if ( ! ( countryList = (char**) calloc (*numCountries + 1, sizeof(char*))))
    	{
    		printf("Memory is unavailable.\n");
    		exit(103);
    	}
    	for ( i = 0; i < *numCountries; i++ )
    	{
    		fgets(tempAry, sizeof(tempAry), spIn);
    		countryList[i] = (char*) calloc (strlen(tempAry), sizeof(char));
    		strcpy(countryList[i], tempAry);
    		printf("%s", countryList[i]);
    	}
    
    	countryList[i] = (char*) calloc (strlen(tempAry), sizeof(char));
    	countryList[*numCountries] = '\0';
    
    	closeResult = fclose ( spIn );
    	if (closeResult==EOF)
    	{
    		printf("File did not close properly.\n");
    		exit(104);
    	}
    	
       return countryList;
    } // getNames

  13. #13
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,185
    So writing past the end of the heap is going to be an error in the writing part of the whole deal, and looking there, yes you manage to calloc one less character than you need every single time through. So fix that.

  14. #14
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,185
    Also, this:
    Code:
    	countryList[i] = (char*) calloc (strlen(tempAry), sizeof(char));
    	countryList[*numCountries] = '\0';
    is a guaranteed memory leak, since i and *numCountries will be the same at this point -- you calloc some memory and then set the pointer to NULL. If you want the pointer to be NULL then don't calloc any memory (and then you don't have to free it back in main). If you want the pointer to exist, but point to an empty string, then you need to set countryList[i][0] to '\0' instead.

  15. #15
    Registered User
    Join Date
    Nov 2008
    Posts
    21
    I didn't know '/n' counted as a character as well, which was part of the reason why I didn't have that extra character space. Thanks for your input, there are no more memory leaks now.

Page 1 of 2 12 LastLast
Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Memory leak with detached pthreads - how to free?
    By rfk in forum Linux Programming
    Replies: 2
    Last Post: 08-17-2007, 06:50 AM
  2. Replies: 2
    Last Post: 09-28-2006, 01:06 PM
  3. How to detect a memory leak?
    By g4j31a5 in forum C++ Programming
    Replies: 11
    Last Post: 07-19-2006, 11:35 AM
  4. Memory Leak?
    By BigDaddyDrew in forum C++ Programming
    Replies: 11
    Last Post: 12-09-2002, 03:28 PM
  5. Is it necessary to write a specific memory manager ?
    By Morglum in forum Game Programming
    Replies: 18
    Last Post: 07-01-2002, 01:41 PM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21