0

I'm attempting to solve the Recover problem, and I'm using a for loop to go through the bytes of the raw file. It spits out 43 images, some of which have the full image, but most are corrupted, and don't have an image at all.

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

int main(int argc, char *argv[])
{
char *fname;
uint8_t buff[512];

if (argc != 2)
{
    printf("Enter the name of the file and only that\n");
    return 1;
}
else
{
    fname = argv[1];
}
//printf("%s", fname);
FILE *infile = fopen(fname, "r");
if (infile == NULL)
{
    printf("Error opening the file\n");
    return 2;
}
fseek(infile, 0, SEEK_END);
long int flength = ftell(infile); // flength holds length of input file in bytes
//printf("%ld", flength);
fseek(infile, 0, SEEK_SET); // Setting the seek back to the start

//uint8_t *buff = malloc(512);
char name[8];
//name = "000.jpg";
int n = 000;
for (int i = 1; i <= flength; i++)
{
    int j = 0;
    //uint8_t *buff = malloc(512); // Creating the buffer block
    //buff = 0;
    fread(buff, 512, 1, infile); // Reading 512 bytes from infile

    if (buff[0] == 0xff && buff[1] == 0xd8 && buff[2] == 0xff && (buff[3] == 0xe0 || buff[3] == 0xe1 || buff[3] == 0xe2 || buff[3] == 0xe3)) // If the headers of a jpeg are found...
    {
        sprintf(name, "%i.jpg", n);
        FILE *outfile = fopen(name, "w"); // Open an output file
        fwrite(buff, 512, 1, outfile);   // Writing the first 512 block to the file
        fread(buff, 512, 1, infile);     // Reading the next 512 block to buffer
        while (buff[0] != 0xff && buff[1] != 0xd8 && buff[2] != 0xff) // If the next 512 byte block didn't contain jpeg headers...
        {
            fwrite(buff, 512, 1, outfile);     // ..then write the 512 block to the output file
            fread(buff, 512, 1, infile);       // Read the next 512 byte block for evaluation by the while condition
            i += 512;                           // Increasing counter i by 512 bytes, since i is limited by the bytes of infile, and each time
        }                                       // this while loop executes the seek has gone forward by 512 bytes

        fclose(outfile);                        // Closing the outfile
        n += 1;                                 // Increasing the counter that handles the outfile name
        //sprintf(name, "%i.jpg", n);             // SPrinting the int counter to the string var that is the outfile name
        //printf("%s", name);                     // Testing
    }
    else
    {
        i += 512;   // Since i is limited by the no of bytes in infile, if the block doesn't contain a header, move on by 512 bytes in flength
    }
    //free(&buff);
}

fclose(infile);
//free(&buff);
return 0;
}

Now what can be the problem? I can't find any, I've probably tunnel visioned being stuck on this for a day. Any help?

Edit:

char name[8];
int n = 000;
while (fread(buff, 512, 1, infile) == 1)
{
    if (buff[0] == 0xff && buff[1] == 0xd8 && buff[2] == 0xff && (buff[3] & 0xe0) == 0xe0)
    {
        if (outfile != NULL)
        {        // line 39
            fclose(outfile);
            n += 1;         //line 41
        }
        sprintf(name, "%i.jpg", n);
        FILE *outfile = fopen(name, "w");
        fwrite(buff, 512, 1, outfile);
    }
    else if (buff[0] != 0xff && buff[1] != 0xd8 && buff[2] != 0xff && (buff[3] & 0xe0) != 0xe0)
    {
        fwrite(buff, 512, 1, outfile);    //line 49
    }
}

This is my new code.

recover6.c:39:17: error: use of undeclared identifier 'outfile'
        if (outfile != NULL)
            ^
recover6.c:41:24: error: use of undeclared identifier 'outfile'
            fclose(outfile);
                   ^
recover6.c:50:34: error: use of undeclared identifier 'outfile'
        fwrite(buff, 512, 1, outfile);

                             ^

These are the new errors that pop up...

1

There are several issues in this code.

First, look at how i is being incremented. Inside the code, it is incremented by 512 in several places, but it's also being incremented by 1 in the for loop setup. That will throw the alignment to 512 byte blocks off.

The test for signatures occurs in 2 places. In the first,

if (buff[0] == 0xff && buff[1] == 0xd8 && buff[2] == 0xff && (buff[3] == 0xe0 || buff[3] == 0xe1 || buff[3] == 0xe2 || buff[3] == 0xe3)) // If the headers of a jpeg are found...

the test is flawed. It doesn't check for 0xE4 through 0xEF. Tip: there's a way to "and" the 4th byte with something to check it against a single value that will handle checking all 16 valid values.

In the second,

while (buff[0] != 0xff && buff[1] != 0xd8 && buff[2] != 0xff) // If the next 512 byte block didn't contain jpeg headers...

this code doesn't check buff[3] at all. Further, if only one of the 4 bytes matchesm but not all 4, the test fails. This could generate false negatives and trigger an exit from the loop.

Complex logic tests like this can't be reversed by simply changing == to !=. The simple way to do it is to enclose the whole test in parentheses and negate it with the NOT operator, !, ie, !(buff[0] == 0xff && buff[1] == 0xd8 && buff[2]...)

There may be more issues, but these jumped out at me.

You might want to rethink the code to see what can be simplified. For example, since the code is doing a simple, linear read of the input file, most people approach it by testing for the end of the input file instead of first determining the size of the file. (ie, a while loop instead of a for loop.) Also, the code to write the data block can be separated from the code that closes one output file and opens a new one. Just some thoughts on different approaches.

Remember the golden rule of programming - KISS Keep It Simple, .... ;-)

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

0

You must log in to answer this question.

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