0

I'm busy with problem set 4, and I can't figure out why my code doesn't work. What seems odd is that the first line or two of a new jpeg will seem partially or fully correct, but thereafter the images seem to become corrupted in some way. The current programme also only outputs 16 images, instead of the 50 required by the problem, but I don't know why it does this. I would really appreciate any help with the issue. Below is my code, thank you!

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

int main(int argc, char *argv[])
{
    //Ensures only 2 command line arguments
    if (argc != 2)
{
    fprintf(stderr, "Usage: ./recover card.raw\n");
    return 1;
}

//Stores filename
char *infile = argv[1];

FILE *inptr = fopen(infile, "r");
if (inptr == NULL)
{
    fprintf(stderr, "%s.\n", infile);
    return 2;
}

//Creates counter for the filename of Jpegs
int counter = 0;

//Creates array to store filenames
char filename[8];

//Allocates memory for 512 bytes to be read
int blocksize = 512;
typedef uint8_t  BYTE;
BYTE *buffer = malloc(blocksize);

//Creates file pointer
FILE *img = NULL;

do
{
    //Reads block into temporary storage
    fread(buffer, 1, blocksize, inptr);

    //Identifies if block is the start of a new jpeg
    if (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[3] & 0xf0) == 0xe0)
    {
        //Creates two different paths based on whether a jpeg is first or after first
        //First jpeg path
        if (counter == 0)
        {
            //Creates new filename
            sprintf(filename, "%03i.jpg", counter);

            //Opens file for writing
            img = fopen(filename, "w");
            if (img == NULL)
            {
                fprintf(stderr, "%s.\n", filename);
                return 3;
            }

            //Writes block into outfile
            fwrite(buffer, 1, blocksize, img);
        }

        //Not first jpeg path
        else
        {
            //Closes previous jpeg
            fclose(img);

            //Creates new name
            sprintf(filename, "%03i.jpg", counter);

            //Opens new jpeg
            img = fopen(filename, "w");
            if (img == NULL)
            {
                fprintf(stderr, "%s.\n", filename);
                return 3;
            }

            //Writes block into outfile
            fwrite(buffer, 1, blocksize, img);
        }

        //Increments counter
        counter++;
    }

    //Identifies if block is end of file
    else if (fread(buffer, 1, blocksize, inptr) != 512)
    {
        fclose(img);
        fclose(inptr);
        free(buffer);
        return 0;
    }

    //Otherwise prints block to current open file
    else if (counter != 0)
    {
        fwrite(buffer, 1, blocksize, img);
    }

}
while(fread(buffer, 1, 512, inptr) == 512);

}
1

You have multiple fread, each reading 512 bytes, but not every read sector seems to be written even after opening a file. This means you're skipping some sectors.

Instead of a do..while loop, use a regular while, and remove any fread in the body, keep only the one in the condition. This will take care of reading and checking for input file end at the same time.

Clean-up should happen after the loop, and consider the case your input file did not contain any JPEG signature (so fclose(img) would lead to a segmentation fault)

2
  • Thank you so much! I didn't realise that if fread is used in a condition, that it reads new information. I thought it would merely check whether the previous fread fulfilled the condition, but obviously that was silly of me. I've gotten the code to work now; thank you so much for the help! – Nick Brinkmann Jun 11 '18 at 9:31
  • There's no way to tell fread, and it won't be able to guess. If you want, you can use an assignment, like size_t bytes_read = fread(buffer, 1, 512, inptr); and test bytes_read, even in an expression (an assignment in C is an expression evaluating to the assigned value) like size_t bytes_read; while ((bytes_read = fread(buffer, 1, 512, inptr)) == 512). Also, after a failed fread, feof(inptr) would return something non-zero. But here, with a head-controlled loop, we don't need those. – Blauelf Jun 11 '18 at 11:39

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .