Search code examples
cpointersmallocfreefread

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?


Solution

  • 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]);
        }