Segmentation fault in recover pset4

Question

I am working on recover in problem set 4. However, after trying out different stuff, the code itself compiles but I get a segmentation fault and the outcome is only one jpg file called 000.jpg. I am lurking on the forum to find a sollution that would help but so far I have had zero success.

// Opening an external file to read
FILE *file = fopen(argv[1], "r");

// Checking for NULL values
if (file == NULL)
{
    // If any, print an error message
    fprintf(stderr, "Could not open, try again!\n");
    return 2;
}

FILE *img = NULL;

// A counter for the file names of the restored JPEG images
int counter = 0;

// An unsigned char is used because of the if check
unsigned char *buffer = malloc(512);

// A boolean to check if the start of a new JPEG file is found
bool jpgStartFinder;

// If the size of a JPEG block is not 512, this would be the end of the loop and the files
// Also reads 512 bytes into the buffer
while(fread(buffer, 512, 1, file) == 1)
{

    // A check for to see if the bytes represent the start of a JPEG image
    if (buffer[0] == 0xff &&
        buffer[1] == 0xd8 &&
        buffer[2] == 0xff &&
        (buffer[3] & 0xf0) == 0xe0)
    {

         // An array serving for a temporary storage for the filename
         char filename[8];

        // A check to see whether the program has already found a JPEG
        if (jpgStartFinder == false)
        {
            // If it has not found a JPEG before, it will start writing a new one

            // Creating a jpg file with the name corresponding to the counter
            sprintf(filename, "%03i.jpg", counter);

            // Opening the file for writing
            img = fopen(filename, "w");

            // Plus one to the counter so that the next JPEG file is with a different name
            counter++;

            // Reading files from buffer and writing them in the array for writing files
            fwrite(buffer, *buffer, 1, img);

        }

        else if (jpgStartFinder == true)
        {
            // If it has found a JPEG before, close the previous file and start writing a new one
            fclose(img);

            // Creating a jpg file with the name corresponding to the counter
            sprintf(filename, "%03i.jpg", counter);

            // Opening the file for writing
            img = fopen(filename, "w");

            // Plus one to the counter so that the next JPEG file is with a different name
            counter++;

            // Reading files from buffer and writing them in the array for writing files
            fwrite(buffer, 512, 1, img);

        }

        jpgStartFinder = true;

        //// Freeing the memory allocation to prevent memory leak
        free(buffer);
        }

    // Not the start of a JPEG file
    else
    {
        // A check to see whether the program has already found a JPEG
        if (jpgStartFinder == false)
        {
            // If it has not found a JPEG before, discard the 512 bytes and go to the start of the loop
            continue;

        }

        else if (jpgStartFinder == true)
        {

            // If it has found a JPEG before, write the bytes here
            // Reading files from buffer and writing them in the array for writing files
            fwrite(buffer, 512, 1, img);
        }
    }
}
// Closing the card
fclose(file);
}

Thanks in advance!

Answer 1

I glad you find out by yourself. It's a common mistake made by experienced developers too. The mistake is called use-after-free -- a kind of temperal memory access violation. As a matter of fact, if you get a segfault, it means your code has a memory access violation. You might want to focus on all the statments that access memory thru pointers first.

This is the incomplete list of the pattern that a likely used in your own code:

• allocate memory on global, on stack, and on heap (this most common situations for beginners, advanced developers have other ways to allocate memory)

    // allocate a global array
    static int x[10];  //static is optional, it means it can only visiable to the current file.

    f () { 
      // x is an array of 10 integers and allocated on the call stack)
      int x[10]; 
    }

    // x is an array of 10 integers and allcated on heap
    int *x = malloc(10*sizeof(int)); 

• access memory like this

    *x = ?; // write to x
    ? = *x; // read from x
    x[i] = ?; // write to x[i]
    ? = x[i];  // read from x[i]

• free memory

     f () { 
       int x[10]; return; // this will actually free the array x 
     }

     // free an array that is allocated on heap 
     free(x) 

These are the statements that you want to focus on when you trouble shoot segfault first. The most complicate situation is many C standard (or third party library) functions that access invalid memory. This is hard for people who are new to C. In this situation you can only rely on tools like gdb or valgrind. I also built an online testing tool for solely testing segfault code, you might want to give it a try at https://stensal.com/demystify_segfault Some common sefault code snippets are pinned, you might want to get familiar with them such that when you come over again, you know how to narrow down your trouble shooting scope. It's a skill you will learn from trial and error.

Answer 2

After thoroughly examing my code and what everything does, I found out that what triggered my segmentation fault was the freeing of memory allocation. It is supposed to be outside of the loop.