Getting segmenatation fault even though i freed memory-CodePudding

#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>
typedef uint8_t BYTE;

int main(int argc, char *argv[])
{
    // Only allow 2 command line arguments
    if (argc != 2)
    {
        printf("Usage: ./recover FILENAME\n");
        return 1;
    }


    // Open the file
    FILE *input_file = fopen(argv[1], "r");

    // Check if the input_file file is a valid ifle
    if (input_file == NULL)
    {
        printf("No existing file");
        return 1;
    }

    // Make a buffer for storing 512 bytes, !unsigned because our values MUSTN'T be negative!
    unsigned char buffer[512];

    // Count the number of images we found
    int image_count = 0;

    // Make a pointer for recovered images
    FILE *output_file = NULL;

    // Use malloc for memory allocation
    char *filename = malloc(8 * sizeof(char));

    // Read the blocks of our array
    while (fread(buffer, sizeof(char), 512, input_file))
    {
        // Check for JPEG header hex values
        if (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[3] & 0xf0) == 0xe0 )
        {
            // Set the names of the JPEG files
            sprintf(filename, "i.jpg", image_count);

            // Open the file we will write to
            output_file = fopen(filename, "w");

            // Count the number of images we found
            image_count  ;
        }
        // Check validity of input output_file input
        if (output_file != NULL)
        {
            fwrite(buffer, sizeof(char), 512, output_file);
        }
        // Free the memory becuase we used malloc (to prevent leaking/segmentation fault)
        free(filename);
        // Close both the input and output files
        fclose(output_file);
        fclose(input_file);
        return 0;
    }
}

I am trying to solve this CS50 problem set and i don't get why I'm getting segmentation fault even though i freed memory. I obviously did something wrong, but i can't wrap my head around what exactly because I am only a begginer in c, especially with all this malloc stuff. Also, I can compile my code, it's only that I get an error when running the program.

CodePudding user response：

You're freeing inside the loop. i.e. you allocate once but try to free it several times.

You want to free it just once. Put it outside the loop.

OTOH, since you know the filenames are only 7 bytes long, you might as well use a char array instead of mallocing.

char filename[256];

Then use snprintf:

snprintf(filename, sizeof filename, "i.jpg", image_count);