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

int main(int argc, char *argv[])
{
    // ensure proper usage
    if (argc != 2)
{
    fprintf(stderr, "Usage: ./recover file\n");
    return 1;
}

char *corruptfile = argv[1];

FILE *corruptfileptr = fopen(corruptfile, "r");
if (corruptfileptr == NULL)
{
    fprintf(stderr, "Could not open %s.\n", corruptfile);
    return 2;
}

unsigned char buffer[512];
int NumOfElements = 512;
int JpegNum = 0;
char filename[8];

//While the header is not found keep reading through the file
while (buffer[0] != 0xff && buffer[1] != 0xd8 && buffer[2] != 0xff && (buffer[3] & 0xf0) != 0xe0)
{
    int EOFcheck = fread(buffer, 1, 512, corruptfileptr);
    if (EOFcheck != 512)
    {
        fclose(corruptfileptr);
        return 3;
    }
}

//When the header is found if header found then make new file else write to it
while (NumOfElements == 512)
{
    NumOfElements = fread(buffer, 1, 512, corruptfileptr);

    if (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[3] & 0xf0) == 0xe0)
    {
        sprintf(filename, "%03d.jpg", JpegNum);
        JpegNum++;

        FILE *img = fopen(filename, "w");

        fwrite(buffer, 512, 1, img);
        fclose(img);

    }
    else
    {

        FILE *img = fopen(filename, "a");
        fwrite(buffer, 512, 1, img);
        fclose(img);
    }
}

fclose(corruptfileptr);
return 0;

}

Hi,

I've completed the code for recover and I can't seem to figure out why I'm missing the last image (it creates a corrupted file in the directory). Also check50 doesn't pass for the 1st image or middle images, despite the images looking alright. Could someone point me in the right direction? Thanks.

(Also feel free to comment on my code's style; implementation may be sloppy!)

EDIT: The block of code I can't seem to get my head around:

    FILE *img = NULL;

while (fread(buffer, 1, 512, corruptfileptr) == 512)
{
    if (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[3] & 0xf0) == 0xe0)
    {

        if (img != NULL)
        {
            fclose(img);
        }
        sprintf(filename, "%03d.jpg", JpegNum);
        JpegNum++;

        img = fopen(filename, "w");

        fwrite(buffer, 512, 1, img);

    }
    else if (JpegNum > 0)
    {
        img = fopen(filename, "a");
        fwrite(buffer, 512, 1, img);
    }
}

fclose(img);
0

Your first condition is wrong. Negating buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[3] & 0xf0) == 0xe0 requires either a common ! and ==, or using || with !=:

!(buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[3] & 0xf0) == 0xe0)

Applying De Morgan's Laws:

buffer[0] != 0xff || buffer[1] != 0xd8 || buffer[2] != 0xff || (buffer[3] & 0xf0) != 0xe0

Also, you'd have to write the block before fread, or at least maintain the sequence read->EOFcheck->write.

I don't like your frequent fopen/fclose. You could leave the file open (img declared in an outer scope, outside of the loop), and if one is open (JpegNum > 0), fclose it before opening the next output file and after the loop.

I would recommend changing the code so that you have only one fread. Could be done for example like while (fread(buffer, 1, 512, corruptfileptr) == 512). Then, inside that loop, you would handle that block depending on whether it represents a JPEG header, and after that whether there's an open output file.

The benefit would be that you don't need the negated header condition at all.

7
  • Thanks, I've updated my logic as per the walkthrough video and it seems to pass all the checks. However, I can't seem to get my program to work without the frequent opening and closing of the file - I tried to do this by declaring FILE *img = NULL; outside of the while loop, and inside of the loop, when a header is found, if img != NULL, then fclose(img);. After the while loop I also fclosed img. Running the program seems to corrupt the images and they come out with weird colours and are pixelated. Could you give me a hint on how to implement this? – XTroyZe Mar 12 '18 at 16:40
  • This sounds more like an issue of the blocks you write and less like an issue of open/close. The if (img != NULL) { fclose(img); } looks great, given you initialised img to NULL. I hope you don't have a second FILE *img left in the loops, that one might do "shadowing". On the content, whenever you read a sector, and there's an output file, write the sector to the file. But first check whether you have to switch the output file, so that block goes to the right JPG. – Blauelf Mar 12 '18 at 17:02
  • Is there a way to post my new code? (EDIT: posted it in the question) I can't seem to figure out where my logic is flawed. I'm updating img by doing img = fopen(filename, "w"); (and for appending). I think it might be due to the sequence/order of operations but I don't see it at the moment. – XTroyZe Mar 12 '18 at 17:59
  • Remove img = fopen(filename, "a"); – Blauelf Mar 12 '18 at 18:22
  • Thank you! I didn't realise that was an unnecessary line with all the code going on. I'm really sorry to keep bothering you but could you explain why opening jpgs to write/append without closing them causes the files to become corrupt? I'd just like to understand it a little more. – XTroyZe Mar 12 '18 at 20:36

You must log in to answer this question.

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