C problem of freeing memory, valgrind error

Question

Hey I'm doing a project about Olympic database that keeps record of medals. Now I have a problem with freeing memory (Valgrind error) and I have no clue how I should free the memory?

Valgrind error comes from the lines:

data[i].country = malloc(strlen(str) + 2);

add_country(countrydata, countryname, i);

First function tries to add country names to the database

    typedef struct Olympia
    {
        char* country;
        int gold;
        int silver;
        int bronze;
    }Olympia;


int add_country(struct Olympia* data, char* str, int i)
    {   
        if (str[0] == '\0') //checking that input is correct
        {
            printf("Error! Try again!\n");
        }

        else
        {
            data[i].country = malloc(strlen(str) + 2);  //allocating memory for country name
            strcpy(data[i].country, str);   //adding country to database
            data[i].gold = 0;   //setting medals to zero
            data[i].silver = 0;
            data[i].bronze = 0;
            i++;
            printf("Country added to database succesfully!\n");
        }
        return i;
    }

Then there is the main function

    int main(void)
    {
        char command;
        int gold = 0;
        int silver = 0;
        int bronze = 0;
        int i = 0;
        char filename[100];

        char* line = (char*)malloc((100) * sizeof(char)); //allocating memory for one stdin line
        char* countryname = (char*)malloc(20 * sizeof(char)); // allocating memory for country name

        struct Olympia* countrydata = malloc(sizeof(struct Olympia) * 1); //allocating memory for structure



        while(1)
        {
            fgets(line, 100, stdin); //reading one line of stdin

            if (feof(stdin) != 0)
            {
                printf("File processing completed!\n");
                free(line);
                free(countryname);
                free(countrydata);
                return 0;
            }

            switch (line[0]) //finding the right command
            {
                case 'A':
                    if (sscanf(line, "%c %s", &command, countryname) == 2)
                    {
                        add_country(countrydata, countryname, i);
                        i++;
                        countrydata = realloc(countrydata, sizeof(struct Olympia) * (i + 1));
                    }
                    else
                    {
                        printf("Error! Invalid input, try again!");
                    }
                    break;
            case 'Q':
                free(line);
                free(countryname);
                free(countrydata);
                return(EXIT_SUCCESS);

            default:
                printf("Error! Invalid input.\n");              
        }   
    }
}

Why the `+2` for `malloc`? Why don't you use `strdup`? And what exactly is the valgrind error? You do not provide it. — Werner Henze, Apr 18 '19 at 12:39
odd using malloc for temp buffers, traditionally stack is used, as you did for the file name. `free(countrydata);` is also not correct, you need to go through the array and free country in all entries before freeing the array itself. — AndersK, Apr 18 '19 at 12:48

score 1 · Accepted Answer · answered Apr 18 '19 at 12:45

I think the problem comes from

data[i].country = malloc(strlen(str) + 2);  //allocating memory for country name

Which is never freed.

To correct this, modify the Q statement:

case 'Q':
{
    int j;
    for (j = 0; j < i; ++j)
    {
        free(countrydata[i].countryname);
    }
    free(line);
    free(countryname);
    free(countrydata);
    return(EXIT_SUCCESS);    
}

But your code has other problem:

you don't test for *alloc functions return,
you use feof which is counter productive (testing the fgets return is enough)
you use dynamic memory where static is enough (line, countryname)

Thanks for the answer. I will check those other problems also. :) — L0KiZ, Apr 18 '19 at 13:12

Diodacus · Answer 2 · 2019-04-18T12:45:35.153

0

You are trying to use data function parameter in add_country function as if it was an array, yet you create only one variable:

struct Olympia* countrydata = malloc(sizeof(struct Olympia) * 1);

This does create variable not an array. Try to increase number in malloc to sizeof(struct Olimpia) * 10 to get 10-element array.

Tip.: Read about lists.

UPDATE Please take a look at proper usage of realloc function - this may cause problems. realloc function may return NULL when not succeeded.

edited Apr 18 '19 at 12:45

answered Apr 18 '19 at 12:40

Diodacus

663
3
8

1

he uses realloc to increase the array – AndersK Apr 18 '19 at 12:42
Right, I didn't see it. – Diodacus Apr 18 '19 at 12:44

C problem of freeing memory, valgrind error

2 Answers2