For some reason I end up with memory leak...

I'm not sure how I'm freeing the memory incorrectly. I freed all the memory in each string until null, but it won't let me free the next level of pointers.....

Code:

---

/*****************************************************************
** CIS 15 BG
** Fall, 2008
***************
**
** Lab 7            Structures          20 Points
**
**************************************************

  This program uses an array of country structures
  to answer inquiries.

****************************************
**
**  Written By:  Ryu Komiyama
**
**  Date: November 23, 2008
**
******************************************************************/
#include <stdio.h>
#include <crtdbg.h>
#include <stdlib.h>
#include <string.h>
#include <ctype.h>
#define FLUSH while (getchar( ) != '\n')
#define FILE_IN "countries.txt"

// Type Definitions
typedef struct {
        char *id;
        char *name;
        char *capital;
        char *population;
        } COUNTRY;

// Prototype Declarations
char getOption ( void );
void inputFile ( COUNTRY *list, int *numCountries );
char **createDynAry ( int numCountries );
void processOption ( char option, COUNTRY *list, int numCountries );
void insertCommas ( COUNTRY *pWalker, COUNTRY *pLast );
void printList ( COUNTRY *list, int numCountries );
void searchID ( COUNTRY *list, int numCountries );
void searchCapital ( COUNTRY *list );
void printPop ( COUNTRY *list, int numCountries );
void releaseMemory ( COUNTRY *list );

int main ( void )
{
//  Local Definitions
        char option = ' ';
        COUNTRY countryList[30];
        int numCountries = 0;

//  Statements
    printf("\t\t LAB 7 - STRUCTURES and SEARCHING\n\n");

    inputFile( countryList, &numCountries );
        while ( option != 'Q' )
        {
                option = getOption ( );
                if ( option == 'Q' )
                        break;
                processOption ( option, countryList, numCountries );
        }
        // free memory here!
        releaseMemory ( countryList );
       

        printf("\n\t\tEnd of LAB 7"
                  "\n\t\tHave a great day!\n");

    printf( _CrtDumpMemoryLeaks() ? "Memory Leak\n" : "No Leak\n");

    return 0;
}// main

char getOption ( void )
{
// Local Declarations
        char option = '\0';

// Statements
        printf("\n\t\tCountry List Menu\n");
        printf("L - list the countries in ID sequence\n");
        printf("P - list the countries by population in descending order\n");
        printf("S - Search by ID\n");
        printf("C - Search by capital\n");
        printf("Q - Quit the program\n\n");
        printf("Please enter an option and then hit enter: ");
        while ( !option )
        {
                scanf("&#37;c", &option);
                FLUSH;
                if ( toupper(option) == 'L' || toupper(option) == 'P' ||
                        toupper(option) == 'S' || toupper(option) == 'C' ||
                        toupper(option) == 'Q')
                        printf("You have selected option \"%c\"\n", toupper(option));
                else
                {
                        option = '\0';
                        printf("That option does not exist. Please try again.\n");
                }
        }
  return toupper(option);
}

void inputFile ( COUNTRY *list, int *numCountries )
{
// Local Declarations
        FILE *spIn;
        COUNTRY *pWalker;
        COUNTRY *pLast;
        char **tempInt, **tempName, **tempCap, **tempPop;
        int i = 0;
// Statements
        if ( ! ( spIn = fopen ( FILE_IN, "r" ))) {
                printf("Unable to access input file.\n");
                exit(100);
        }
        fscanf( spIn, "%d", numCountries );
        pLast = list + *numCountries - 1;
        tempInt = createDynAry ( *numCountries );
        tempName = createDynAry ( *numCountries );
        tempCap = createDynAry ( *numCountries );
        tempPop = createDynAry ( *numCountries );
        for ( pWalker = list; pWalker <= pLast; pWalker++ ) {
                if ( ! ( tempInt[i] = ( char* ) calloc ( 3, sizeof ( char )))) {
                        printf("Memory unavailable.\n");
                        exit(200);
                }
                if ( ! ( tempName[i] = ( char* ) calloc ( 21, sizeof ( char )))) {
                        printf("Memory unavailable.\n");
                        exit(200);
                }
                if ( ! ( tempCap[i] = ( char* ) calloc ( 16, sizeof ( char )))) {
                        printf("Memory unavailable.\n");
                        exit(200);
                }
                if ( ! ( tempPop[i] = ( char* ) calloc ( 16, sizeof ( char )))) {
                        printf("Memory unavailable.\n");
                        exit(200);
                }
                fscanf( spIn, "%2s %20[^;] %*c %15[^;] %*c %s",
                                tempInt[i], tempName[i], tempCap[i], tempPop[i]);
                pWalker->id = tempInt[i];
                pWalker->name = tempName[i];
                pWalker->capital = tempCap[i];
                pWalker->population = tempPop[i];
                i++;
                insertCommas ( pWalker, pLast );
        }
        pWalker->id = '\0';
        pWalker->name = '\0';
        pWalker->capital = '\0';
        pWalker->population = '\0';
  return;
}

char **createDynAry ( int numCountries )
{
// Local Declarations
        char **ary;
// Statements
        if ( ! ( ary = ( char** ) calloc ( numCountries, sizeof ( char* )))) {
                printf("Memory unavailable.\n");
                exit(200);
        }
        ary[numCountries] = '\0';
        return ary;
}

void processOption ( char option, COUNTRY *list, int numCountries )
{
// Statements
        switch ( option )
        {
        case 'L':        printList( list, numCountries );
                                break;
        case 'P':        printPop ( list, numCountries );
                                break;
        case 'S':        searchID( list, numCountries );
                                break;
        case 'C':        searchCapital( list );
                                break;
        default:        printf("Fatal error. Variable 'option' is corrupt.\n");
                                exit(300);
        }
  return;
}

void insertCommas ( COUNTRY *pWalker, COUNTRY *pLast )
{
// Local Declarations
        char printString[15];
        char tempString[15];
        char insertComma[] = ",";
        int strLength;
        int checkRmdr;
        int i;
// Statements
        *printString = '\0';
        strcpy(tempString, pWalker->population);
        strLength = strlen ( tempString );
        checkRmdr = strLength % 3;
        if ( checkRmdr > 0 )
        {
                strncat ( printString, tempString, checkRmdr );
                strcat ( printString, insertComma );
        }
        for ( i = checkRmdr; i < (int) strlen ( tempString );  )
        {
                strncat ( printString, tempString + i, 3 );
                i += 3;
                strcat ( printString, insertComma );
        }
        printString[strlen(printString) - 1] = '\0';
        strcpy(pWalker->population, printString);
        return;
}

void printList ( COUNTRY *list, int numCountries )
{
// Local Declarations
        COUNTRY *pWalker;
        COUNTRY *pLast;
// Statements
        pWalker = list;
        pLast = list + numCountries;
        printf(        "== ==================== ================ ================\n"
                        "ID NAME                Capital                Population\n"
                        "== ==================== ================ ================\n");
        for ( pWalker = list; pWalker < pLast ; pWalker++ )
        {
                printf("%s %-20s %-15s %17s\n",
                          pWalker->id, pWalker->name,
                          pWalker->capital, pWalker->population);
        }
        printf( "======================= ================ ================\n" );
  return;
}

void searchID ( COUNTRY *list, int numCountries )
{
// Local Declarations
        COUNTRY *pFirst;
        COUNTRY *pLast;
        COUNTRY *pMid;
        char id[3];
        int found = 0;
       
// Statements
        pFirst = list;
        pLast = list + numCountries - 1;
        printf("Enter the ID: ");
        gets(id);
        while ( strcmp(id, pFirst->id ) >= 0 && strcmp(id, pLast->id ) <= 0 ) {
                pMid = pFirst + (pLast - pFirst) / 2;
               
                if ( strcmp ( pMid->id, id ) < 0 )
                        pFirst = ++pMid;
                if ( strcmp ( pMid->id, id ) > 0 )
                        pLast = --pMid;
                if ( strcmp ( pMid->id, id ) == 0 )
                {
                        found = 1;
                        break;
                }
        }
        if ( found ) {
                printf(        "\n        ID: %s\n"
                                "      Name: %s\n"
                            "  Capital: %s\n"
                                "Population: %s\n", pMid->id, pMid->name,
                                                                        pMid->capital, pMid->population);
        }
        else
                printf("No matching country. Please try again.\n");
  return;
}

void searchCapital ( COUNTRY *list )
{
// Local Declarations
        char capital[16];
        COUNTRY *pWalker;
        int found = 0;
       
// Statements
        printf("Enter the capital: ");
        gets( capital );
        pWalker = list;
        while ( pWalker->capital )
        {
                if ( strcmp ( capital, pWalker->capital ) == 0 )
                {
                        found = 1;
                        break;
                }
                pWalker++;
        }
        if ( found ) {
                printf(        "\n        ID: %s\n"
                                "      Name: %s\n"
                            "  Capital: %s\n"
                                "Population: %s\n", pWalker->id, pWalker->name,
                                                                        pWalker->capital, pWalker->population);
        }
        else
                printf("Capital is not in the database. Please try again.\n");
        return;
}

void printPop ( COUNTRY *list, int numCountries )
{
// Local Declarations
        int sorted;
        int walker = 0;
        int last = numCountries - 1;
        int lastValue;
        int nextValue;
        COUNTRY temp;
        int i;
// Statements
        do
        {
                sorted = 1;
                last = numCountries - 1;
                while ( walker < last )
                {
                        lastValue = 0;
                        nextValue = 0;
                        i = 0;
                        while ( list[last].population[i] != '\0' )
                        {
                                lastValue += list[last].population[i];
                                i++;
                        }
                        i = 0;
                        while ( list[last-1].population[i] != '\0' )
                        {
                                nextValue += list[last-1].population[i];
                                i++;
                        }

                        if ( lastValue > nextValue )
                        {
                                temp = list[last];
                                list[last] = list[last-1];
                                list[last-1] = temp;
                                sorted = 0;
                        }
                        last--;
                }
                walker++;
        } while ( !sorted );
        printList(list, numCountries);
  return;
}

void releaseMemory ( COUNTRY *list )
{
// Local Declarations
        int i;

// Statements
        i = 0;
        while ( list[i].id )
                free ( list[i++].id );
        i = 0;
        while ( list[i].name )
                free ( list[i++].name );
        i = 0;
        while ( list[i].capital )
                free ( list[i++].capital );
        i = 0;
        while ( list[i].population )
                free ( list[i++].population );


  return;
}

---

Please
show
error
messages.

Code:

---

          Country List Menu
L - list the countries in ID sequence
P - list the countries by population in descending order
S - Search by ID
C - Search by capital
Q - Quit the program

Please enter an option and then hit enter: q
You have selected option "Q"

                End of LAB 7
                Have a great day!
Memory Leak
Press any key to continue . . .

---

Code:

---

void releaseMemory ( COUNTRY *list )
{
// Local Declarations
        int i;

// Statements
        for ( i = 0; i <= 21; i++ )
        {
                free ( list[i].id );
                free ( list[i].name );
                free ( list[i].capital );
                free ( list[i].population );
        }

  return;
}

---

Where does this magic 21 come from? free()'ing non-NULL pointers that you didn't allocate is very bad.

Also don't use gets() -- see the FAQ.

Quote:

---

Originally Posted by zacs7

Code:

---

void releaseMemory ( COUNTRY *list )
{
// Local Declarations
        int i;

// Statements
        for ( i = 0; i <= 21; i++ )
        {
                free ( list[i].id );
                free ( list[i].name );
                free ( list[i].capital );
                free ( list[i].population );
        }

  return;
}

---

Where does this magic 21 come from? free()'ing non-NULL pointers that you didn't allocate is very bad.

Also don't use gets() -- see the FAQ.

---

I just put in 21 to see whether or not I freed the memory correctly. 21 is numCountries -1. The list's variables carry pointers to tempInt, tempName, tempCap, and tempPop. I also don't like using fgets in this situation because it carries over the '\n' as well.

Okay, I fixed releaseMemory to be more NULL friendly.

Code:

---

void releaseMemory ( COUNTRY *list )
{
// Local Declarations
        int i;

// Statements
        i = 0;
        while ( list[i].id )
                free ( list[i++].id );
        free ( list[i].id );
        i = 0;
        while ( list[i].name )
                free ( list[i++].name );
        free ( list[i].name );
        i = 0;
        while ( list[i].capital )
                free ( list[i++].capital );
        free ( list[i].capital );
        i = 0;
        while ( list[i].population )
                free ( list[i++].population );
        free ( list[i].population );

  return;
}

---

1. Pass numCountries to your release function.
2. Call inputFile( countryList, &numCountries ); only ONCE.

You're allocating space for the whole file every time around the loop, so unless the first thing you do is quit, it's leaking.

Quote:

---

Originally Posted by Salem

1. Pass numCountries to your release function.
2. Call inputFile( countryList, &numCountries ); only ONCE.

You're allocating space for the whole file every time around the loop, so unless the first thing you do is quit, it's leaking.

---

As long as I free the memory in a while loop 'til it hits NULL then I wouldn't need numCountries right? I'm not sure if releaseMemory is freeing the first level dynamically allocated pointers or the second level pointers...

That's fine if you don't want to use fgets() -- the FAQ explains how to remove the \n. Just DON'T use gets(). You're post is about a memory leak, yet you have a potential buffer overrun (which could also cause a memory leak).

> I'm not sure if releaseMemory is freeing the first level dynamically allocated pointers or the second level pointers...
Perhaps you need to brush up on your pointers then...

Well it's under the assumption that the user correctly defines what they want. But anyway...

It's just that structures makes it a little different. I'm okay with pointers but I'm not godlike at it.

Found it. I completely forgot that stacks and heap are different. I removed the following code and made the appropriate changes.

Code:

---

tempInt = createDynAry ( *numCountries );
tempName = createDynAry ( *numCountries );
tempCap = createDynAry ( *numCountries );
tempPop = createDynAry ( *numCountries );

---