pset4 recover - how can I get last few bytes to JPG

Question

I've finally gotten recover.c to the point where it compiles. When I run it, it gives me this error message:

munmap_chunk(): invalid pointer
Aborted

But it also generates all fifty of the pictures! The only thing it doesn't produce is the very bottom of 050.jpg, which ends in several rows of transparency.

This is, I think, a very similar question to one that was asked in 2017: cs50 pset4 recover.c error: munmap_chunk(): invalid pointer But frankly, I don't understand how that was resolved. It's taken me two weeks to even get to the point where it compiles and produces working images. Any help or advice you could give would be very much appreciated. Thanks for your time! The code is below:

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

typedef uint8_t BYTE;

int main(int argc, char *argv[])
{
    // checks for correct number of arguments
    if (argc != 2)
    {
        printf("Correct usage is recover <filename.raw>\n");
        return 1;
    }

    // open a new file
    FILE *in_file = fopen(argv[1], "r");
    if (in_file == NULL)
    {
        printf("Null file / file cannot be opened.\n");
        return 1;
    }

    FILE *out_file = NULL;
    BYTE buffer[512];
    int jpg_num = 0;
    char jpg_name[8];
    int i = 0;

    // this is where the munmap()_chunk issue could be coming from
    // originally: while (fread(buffer, 512, 1, in_file) == 1)
    while ((i = fread(buffer, 1, 512, in_file)) > 511)
    {
        if (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[3] & 0xf0) == 0xe0)
        {
            // close old JPEG, if there
            if (jpg_num > 0)
                fclose(out_file);
            jpg_num++;
            sprintf(jpg_name, "%03i.jpg", jpg_num);

            // open a new JPEG
            out_file = fopen(jpg_name, "w");
            if (out_file == NULL)
            {
                printf("Unable to create file.\n");
                return 1;
            }
        }

        // write current image
        // might be an algorithmic problem in laying it out like this
        if (jpg_num > 0)
                fwrite(buffer, 512, 1, out_file);
    }

    // latest attempt to account for missing bits - doesn't do anything, as far as I can tell
    fwrite(buffer, 1, i, out_file);

    free(buffer);
    free(out_file);
    return 0;
}

Answer 1

There are several cases of "too much" or "this doesn't work here."

First, the invalid pointer is being caused by free(buffer);. The buffer array is not a pointer to malloc's or otherwise runtime allocated memory. It's a regular variable, an array. Standard cleanup routines in C will take care of it.

Second, the 50 files are numbered 0 to 49, not 1 to 50.

Third, the final output file needs to be closed with fclose(out_file);, not a call to free(). Again, improper use of free, as before.

Fourth, the final call to fwrite() that follows the while loop is writing the contents of buffer to the output file a second time, so that last write is being executed twice.

Finally, something a lot more subtle. Look at this code:

 while ((i = fread(buffer, 1, 512, in_file)) > 511)

This is mishandling the in_file EOF handling. Specifically, it's the assignment to i. The test is evaluating the success of the assignment of the fread return code to the variable i. It's not comparing the actual return code to 511. This is easily fixed by removing i = from the statement.

Having said that, the better form would be this:

    while ((fread(buffer, 1, 512, in_file)) == 512)

Anything other than 512 indicates an unexpected condition.

If this answers your question, please click on the check mark to accept. Let's keep up on forum maintenance. ;-)