0

I know this question has been asked many times before on this forum, but none of the problems addressed so far seem to be the issue in my program. As for many people, my program seems to be working fine in recovering all 50 JPEGs. But when i run check50, i get the following error: expected exit code 0, not 3. Here's my code:

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

typedef uint8_t  BYTE;

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

    char* image = argv[1];

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

    BYTE block[512]; // block of 512B
    int num_jpeg = 0; // keeps track of total number of jpegs found on SD card
    int offsets[50] = {0}; // stores offsets of JPEGS found on SD card

    while(fread(block, 512, 1, inptr) == 1) // keep reading through file until we can't read a full block anymore
    {
        if(block[0] == 0xff && block[1] == 0xd8 && block[2] == 0xff && (block[3] & 0xf0) == 0xe0) // if we find a JPEG signature (we use a mask to check first
        {                                                                                         // nibble of last byte)
            offsets[num_jpeg] = (ftell(inptr) - sizeof(block));
            num_jpeg++;
        }
    }

    for(int i = 0, j = 1; i < num_jpeg; i++, j++)
    {
        int cur_size = 0;

        if(j < 50)
        {
            cur_size = offsets[j] - offsets[i]; // store the size of the current JPEG
        }
        else if(j == 50)
        {
            fseek(inptr, 0, SEEK_END); // set position indicator to last position
            cur_size = ftell(inptr) - offsets[i]; // get size of last JPEG
        }

        char cur_name[10]; // string to store name of current JPEG
        sprintf(cur_name, "%.3i.jpg", i); // set current name

        BYTE* jpeg_array = malloc(cur_size * sizeof(BYTE)); // create an array large enough to store current JPEG being read
        if(jpeg_array == NULL)
        {
            fclose(inptr);
            fprintf(stderr, "Could not allocate memory for %s.\n", cur_name);
            return 3;
        }

        fseek(inptr, offsets[i], SEEK_SET); // set current position indicator to beginning of JPEG
        fread(jpeg_array, cur_size, 1, inptr); // read the current JPEG from the file

        FILE* outptr = fopen(cur_name, "w"); // open new file to store JPEG in
        if(outptr == NULL) // make sure we can make the file
        {
            fclose(inptr);
            fprintf(stderr, "Could not create %s.\n", cur_name);
            return 4;
        }

        fwrite(jpeg_array, cur_size, 1, outptr); // write the current JPEG into its own .jpg file
        fclose(outptr); // close current JPEG file
        free(jpeg_array); // free jpeg_array
    }

    fclose(inptr);
    return 0;
}

I checked the exit code for the program with echo $? and it is 0 (as it is supposed to be). I also stepped through the program with debug50 and it never enters the body of the if-condition if(jpeg_array == NULL), which it would need to do in order to be able to return 3. I also checked with valgrind to make sure there are no memory leaks or file pointers that aren't properly freed. So i really am at a loss as to why check50 returns the error it does.

I also wrote a different version of the same program in which malloc() is not inside of the for loop (i just allocate a buffer big enough to fit the largest JPEG found). In this program check50 does manage to at least pass the first few tests, but then gives me this error: :( recovers 015.jpg correctly recovered image does not match. So any and all help will be greatly appreciated.

EDIT: The problem has been solved and was obvious in retrospect. Because i cut corners and hardcoded the number of JPEGs, problems would arise when the number of JPEGs on the actual data card would not equal 50 (as is the case with card.raw that check50 uses). Though i got both previous versions of my program to work by accounting for a variable number of possible JPEGs, the design was suboptimal to begin with. So i rewrote the program entirely. For those interested, this is the code of the rewritten program:

#include <stdio.h>
#include <stdint.h>
#include <stdlib.h>
#define FAT_BLOCK 512

typedef uint8_t BYTE;

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

    FILE* inptr = fopen(argv[1], "r"); // open the image
    if(inptr == NULL)
    {
        fprintf(stderr, "Could not open %s.\n", argv[1]);
        return 2;
    }

    BYTE block[FAT_BLOCK]; // block of 512B
    char cur_name[10];
    int num_jpeg = 0; // keeps track of total number of jpegs found on SD card
    FILE* outptr = NULL; // make outptr

    while(fread(block, FAT_BLOCK, 1, inptr) == 1) // keep reading through file until we can't read a full block anymore
    {
        if(block[0] == 0xff && block[1] == 0xd8 && block[2] == 0xff && (block[3] & 0xf0) == 0xe0)
        {
            sprintf(cur_name, "%.3i.jpg", num_jpeg);
            outptr = fopen(cur_name, "w");
            fwrite(block, FAT_BLOCK, 1, outptr);
            fclose(outptr);
            num_jpeg++;
        }
        else if(num_jpeg != 0)
        {
            outptr = fopen(cur_name, "a");
            fwrite(block, FAT_BLOCK, 1, outptr);
            fclose(outptr);
        }
    }

    fclose(inptr);
    return 0;
}
0

You've hardcoded the number of jpgs in the rawfile. Your program needs to handle any raw file given to it. check50's file does not have 50 jpgs, so your program will run into trouble.

Also, because of this, your cur_size value isn't correct when you get to the last jpg, and malloc will fail.

Would be better to write to the file as you go, and not create an array to hold it.

| improve this answer | |
  • You were right on both counts. I know we're not supposed to use magic numbers, but i figured i would just cut a few corners with this particular problem (i also didn't bother to define 512 as a constant). So it just goes to show that cutting corners is a bad idea. I agree the design of my program was not optimal to begin with, it would've been better to just write the files as we encounter the JPEGs. I'm not really sure why i stuck to this implementation with offsets, which is admittedly a bit clunky. Anyways, many thanks! – PvtWitt Dec 4 '17 at 9:34
  • On second thought, i do have one final question. I free the memory i allocate on every iteration of the loop, so the total usage of heap memory at any given time in the program is at most equal to the memory needed for the largest JPEG (plus any open file handles). So how could i possibly exceed the amount of memory available on the test machine? – PvtWitt Dec 4 '17 at 10:54
  • @PvtWitt you are correct. I looked at it more closely. The issue is that cur_size becomes negative because you hard-coded the number of jpgs, which means the malloc will fail. I'll edit my answer. – curiouskiwi Dec 4 '17 at 19:16
  • You are correct, that was exactly the problem and in retrospect i can't believe i missed it. You were also correct that the original setup of the program was not optimal, so in the end i also just rewrote the entire program. Anyways, many thanks for your help! – PvtWitt Dec 5 '17 at 15:56

You must log in to answer this question.

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