Recover used to make files and now doesn't (Spoiler Alert)

Question

It took me a while to figure out why my files wouldn't be created, but eventually, I made a change that did it! I saved and left that session, and when I came back, nothing had changed, but when I ran the program, the files were no longer created. I used debug50, but for some reason, it randomly ends after a few iterations. Here is my code:

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

int main(int argc, char *argv[])
{
    // Ensure that only one argument is inputted; otherwise, end program
    if (argc != 2)
    {
        printf("Usage: ./recover image\n");
        return 1;
    }
    // Open the argument file
    FILE *file = fopen(argv[1], "r");
    // If the file cannot be read, tell the user and end program
    if (file == NULL)
    {
        printf("Your file could not be read.\n");
        return 1;
    }
    // Declare temporary buffer array
    unsigned char temp[512];
    // Initialize variable for naming jpegs
    int i = 0;
    // String variable to store file names, left 8 spaces because file name will always be 7 chars plus the null 0/
    char *filename = malloc(8);
    // Declare a file for new jpgs
    FILE *new_file;
    // While there is still unread memory left in the card
    while (fread(temp, sizeof(unsigned char), 1, file) != 0)
    {
        // Read 512 bytes from the inputted file to the temporary array
        fread(temp, 1, 512, file);
        // If the first 4 bytes signify a jpeg
        if (temp[0] == 0xff && temp[1] == 0xd8 && temp[2] == 0xff && (temp[3] & 0xf0) == 0xe0)
            // If this is the first jpeg
            if (i == 0)
            {
                // Make a name for the new jpeg file
                sprintf(filename, "%03i.jpg", i);
                // Open that jpeg file with the ability to write into it
                new_file = fopen(filename, "w");
                // Write the 512 bytes from temp array into the new jpeg file
                fwrite(temp, 1, 512, new_file);
                // Add one to the jpeg counter
                i++;
            }
            // If this is not the first jpeg
            else
            {
                // Close the file that is currently being written into
                fclose(new_file);
                // Make a name for the new jpeg file
                sprintf(filename, "%03i.jpg", i);
                // Open that jpeg file with the ability to write into it
                new_file = fopen(filename, "w");
                // Write the 512 bytes from temp array into the new jpeg file
                fwrite(temp, 1, 512, new_file);
                // Add one to the jpeg counter
                i++;
            }
        else
        {
            // If a jpeg has already been found
            if (i != 0)
            {
                // Write the 512 bytes from temp array into the already opened jpeg file
                fwrite(temp, 1, 512, new_file);
            }
        }

    }
    if (i != 0)
    {
        fclose(new_file);
    }
    fclose(file);
    free(filename);
}

Answer 1

Let's look at some of the code:

while (fread(temp, sizeof(unsigned char), 1, file) != 0)
{
    // Read 512 bytes from the inputted file to the temporary array
    fread(temp, 1, 512, file);
    // If the first 4 bytes signify a jpeg
    if (temp[0] == 0xff && temp[1] == 0xd8 && temp[2] == 0xff && (temp[3] & 0xf0) == 0xe0)

This code reads 1 byte into temp. Then it reads the next 512 bytes, overwriting the 1 byte that was read.

This puts the reads out of synch with the 512 byte boundaries, moving the read pointer 1 extra byte for every read.

This is a common error by new programmers. Calling a function in an if or while statement doesn't just provide a return value. It actually executes the function, in this case, an fread.

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

This reads 1 byte