My code will only print 23 jpg's which are unreadable, I have been stuck a long time already, xxd'd the jpg's and everything seems to be fine

Question

/**
* recover.c
*
* Computer Science 50
* Problem Set 4
 *  
 * Recovers JPEGs from a forensic image.
 */

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

typedef uint8_t  BYTE;

int jpeg = 0;
int main(int argc, char* argv[])
{
// open the memory card
FILE* inptr = fopen("card.raw", "r");
// open output file
// create output file
char* outfile = "000.jpg";
outfile = malloc(7);
FILE* outptr = NULL;
// check for successful open
if (inptr == NULL)
{
    printf ("Could not open card.raw");
    return 1;
}
// declare buffer as a 1 byte array of 512 values
BYTE buffer[512];
// stop when you have less than 512 values left in card
while (fread(&buffer[0], 512, 1, inptr) == 1)
{
    // read 512 bytes from card and store in buffer
    fread(&buffer[0], 512, 1, inptr);
    // check if the first values correspond to a jpeg
    if (buffer[0] == 0xff &&  buffer[1] == 0xd8 && buffer[2] == 0xff &&             (buffer[3] == 0xe0 || buffer[3] == 0xe1))
    {
        // name the output file according to specifed convention
        sprintf (outfile, "%03d.jpg", jpeg);
        outptr = fopen(outfile, "w");
        // check for error
        if (outptr == NULL)
        {
            fclose(inptr);
            fprintf(stderr, "Could not create %s.\n", outfile);
            return 2;
        }
        // write the 512 values into the new outfile
        fwrite (&buffer[0], 512, 1, outptr);
        // add one to jpeg
        jpeg = jpeg + 1;
        // close outfile
        fclose (outptr);
    }
    // offset pointer 512 bytes to the right in the card
    //fseek(inptr, 512, SEEK_CUR);
}
// close "card.raw"
fclose (inptr);
// free memory from buffer
free (buffer);
return 0;
  }

Answer 1

Aparrently, you didn't look far enough. XXD may have shown you signatures, but a simple ls -al directory listing would have shown you that every file was exactly 512 bytes. That should have been a big red flag.

Your code has at least 5 issues to address.

First is your outfile variable. It is created as a pointer and assigned a string. On the next line, you malloc memory to the pointer (btw, it's too short), changing it from a stack variable to a heap variable. Later, you assign a string to it in the sprintf command - three different techniques that aren't really compatible. All of this is causing your seg faults. You should just allocate it as a char array of length 8 to accommodate the filename plus the end of string marker.

Second, you have free(buffer); at the end of the code. the free() function is only for memory that was previously allocated using malloc or one of its related functions. It is not used on stack variables.

Third, you're looking for two specific signatures that have the 4th byte = 0xE0 and 0xE1. That was last year's version. There are now 16 possible signatures. You need to go back and read the current year's assignment.

http://cdn.cs50.net/2016/x/psets/4/pset4/pset4.html#recover

The fourth byte, meanwhile, is either 0xe0, 0xe1, 0xe2, 0xe3, 0xe4, 0xe5, 0xe6, 0xe7, 0xe8, 0xe8, 0xe9, 0xea, 0xeb, 0xec, 0xed, 0xee, of 0xef. Put another way, the fourth byte’s first four bits are 1110.

Fourth, you are only writing blocks of data that have the signature in the first four bytes. These are signature blocks that mark the start of the file. After the first signature block is found, any blocks without signatures are continuation blocks with image data that have to be copied into the open file. When a new signature is found, then the current file is closed and a new one opened.

Fifth, and probably most critical, you have two back to back freads - one in the while loop setup and the next in the first line of the while loop code. This effectively throws away half of the data, and you're missing 26 of the signature blocks. The fread in the while loop isn't just a test for EOF, it actually executes a file read! The value that fread() returns after doing the read is what tells the while test whether EOF was detected.

No guarantees that there aren't more problems, but this will get you going.

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