CODEWITHSUNDEEP
cpointersmallocfreefread
o11899nine
o11899nine

Reputation: 5

Question about freeing malloc()ed memory in C

I'm doing the CS50 course (so please don't give me the exact right answer, but point me in the right direction!

I got my program (below) to work (although I'm not sure if I did it 'the right way'); it prints the 8 license plates from plates.txt. However, valgrind still tells me I'm losing some bytes. I know for sure that it has to do with my 'temp' thing allocating memory in the loop. I just don't know how to fix it. If anyone could point me in the right direction, that would be wonderful!

Valgrind:

==18649== HEAP SUMMARY:
==18649== in use at exit: 49 bytes in 7 blocks
==18649== total heap usage: 10 allocs, 3 frees, 4,624 bytes allocated
==18649==
==18649== 49 bytes in 7 blocks are definitely lost in loss record 1 of 1
==18649== at 0x4848899: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==18649== by 0x109257: main (license.c:39)
==18649==
==18649== LEAK SUMMARY:
==18649== definitely lost: 49 bytes in 7 blocks
==18649== indirectly lost: 0 bytes in 0 blocks
==18649== possibly lost: 0 bytes in 0 blocks
==18649== still reachable: 0 bytes in 0 blocks
==18649== suppressed: 0 bytes in 0 blocks
==18649==
==18649== For lists of detected and suppressed errors, rerun with: -s
==18649== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

Program code:

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

int main(int argc, char *argv[])
{
    // Check for command line args
    if (argc != 2)
    {
        printf("Usage: ./read infile\n");
        return 1;
    }

    // Create buffer to read into
    char buffer[7];

    // Create array to store plate numbers
    char *plates[8];

    // Create a pointer that will later point to a place in memory on the heap for strcpy
    char *temp = NULL;

    FILE *infile = fopen(argv[1], "r");

    if (infile == NULL)
    {
        printf("File not found.\n");
        return 1;
    }

    int idx = 0;

    while (fread(buffer, 1, 7, infile) == 7)
    {
        // Replace '\n' with '\0'
        buffer[6] = '\0';

        // Allocate memory to temporarily store buffer contents
        temp = malloc(sizeof(buffer));

        // Copy buffer contents to temp
        strcpy(temp, buffer);

        // Save plate number in array
        plates[idx] = temp;
        idx++;
    }

    fclose(infile);

    for (int i = 0; i < 8; i++)
    {
        printf("%s\n", plates[i]);
    }

    free(temp);

    return 0;
}

I fclosed the file and freed my 'temp' location in the heap. However, I malloc() temp multiple times, but I can't free(temp) multiple times?

Upvotes: 0

Views: 136

Answers (2)

0___________
0___________

Reputation: 67476

You need to free all allocated pointers.

for(i = 0; i < idx; i++) 
   free(plates[i]);

You also can very easily fo beyond your array bounds as you do not check the index

 while (idx < 8 && fread(buffer, 1, 7, infile) == 7)

Printing is also wrong as you assume that you have read all 8 chunks.

    for (int i = 0; i < idx; i++)
    {
        printf("%s\n", plates[i]);
    }

Upvotes: 2

Lundin
Lundin

Reputation: 213513

free(temp); is wrong, it just deletes the last item allocated.

Since you called malloc in a loop, you'll also have to call free in a loop. The rule of thumb is that every malloccall must be matched by a freecall.

So you have to make a for loop iterating across char *plates[8]; and free everything that each plates pointer points at.

Upvotes: 4

Related Questions