pset4: what does check50 "TODO" error mean?

Question

I am able to successful save and view the files from the card.raw for PSET4, however the check50 is giving me sad faces for recovers 000.jpg correctly and recovers 001.jpg through 014.jpg correctly. The sandbox (https://sandbox.cs50.net/checks/8af0ae25eb2f481e9fcf55ae3b1e27d0) shows a red "TODO" for these two sad faces. Can anyone please give me a hint as to what I failed "TODO"? Is my code too slow? I already commented out my first block of code because that was causing check50 to give me a "killed by server" error. Thanks!!! Here is my code:

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

#define BLOCK 512

typedef uint8_t BYTE;

int main(int argc, char* argv[])
{
// make sure nothing fishy is going on
if (argc != 1)
{
    printf("improper usage\n");
    return 1;
}

// open file
FILE* file = fopen("card.raw", "r");

// make sure file actually opened
if (file == NULL)
{
    printf("could not open file\n");
    return 2;
}

// initialize variables
// bool found_file = false;
int file_count = 0;
FILE* new_file = NULL;

/*    // find the first jpg file*/
/*    do*/
/*    {    */
/*        unsigned char temp[4];*/
/*        fread(temp, 4, 1, file);*/
/*        */
/*        if (temp[0] == 0xff && temp[1] == 0xd8 && temp[2] == 0xff &&     (   */
/*            temp[3] == 0xe0 || temp[3] == 0xe1))*/
/*        {       */
/*            found_file = true;*/
/*                */
/*            // go back to start of jpeg file info*/
/*            fseek(file, -4, SEEK_CUR);*/
/*        }*/
/*        else*/
/*        {   */
/*            fseek(file, -3, SEEK_CUR);*/
/*        }  */
/*    }*/
/*    while (!found_file);*/

// write each jpg to a new file                
do
{     
    // read in block
    // unsigned char buffer[BLOCK];
    BYTE buffer[BLOCK];
    fread(buffer, BLOCK, 1, file);

    if (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (
        buffer[3] == 0xe0 || buffer[3] == 0xe1))
    {
        // name and open new file
        char file_name[8];
        if (file_count < 10)
        {
            sprintf(file_name, "00%i.jpg", file_count);
        }
        else
        {
            sprintf(file_name, "0%i.jpg", file_count);
        }

        new_file = fopen(file_name, "w");

        // make sure file actually opened
        if (new_file == NULL)
        {
            printf("could not create file\n");
            return 3;
        }

        // record new file was made
        file_count++;

        // write buffer to file
        fwrite(buffer, BLOCK, 1, new_file);

    }
    // if the file is still open write the next block to it
    else if (new_file != NULL)
    {
        fwrite(buffer, BLOCK, 1, new_file);   
    } 

    // if file is open check block for jpg end and close file if found
    if (new_file != NULL)
    {
        for (int i = 0; i < BLOCK; i++)
        {
            if (buffer[i] == 0xff && buffer[i + 1] == 0xd9 && 
                buffer[i + 2] == 0x00 && buffer[i + 3] == 0x00)
            {
/*                    for (int j = i - 2; j < i + 6; j++)*/
/*                    {*/
/*                        printf("0x%02x ", buffer[j]);*/
/*                    }*/
/*                    printf(" \n");*/
                fclose(new_file);
                new_file = NULL;
                break;
            }   
        }
    }
}
while (feof(file) == 0);           

fclose(file); 
return 0;       
}

Answer 1

I don't know what the block that has the for loop:

for (int i = 0; i < BLOCK; i++)

is supposed to do. But in practice, it doesn't do anything useful. You find the signature, you open the file, you write to the file. Then you check that new_file !=NULL (it isn't, of course, you've just opened the file and wrote the first 512 bytes to it). So you go through the for loop and, of course, it will match the signature. It matched it before, there's no reason why it won't match it now. So you close the file you've just opened and set the pointer to null. You probably don't have jpegs, but files that have 512 bytes each.

Also, while (feof(file) == 0) is never a good idea.

feof will only be set after the file has reached the end. By the time you reach it, you have already read and written past the end of the file.

Answer 2

EXPLANATION: It turned out that my code was too generic. Perhaps this caused it to run too slowly for check50 even though both programs run "instantaneously" on my computer (a previous version of mine took about 1 second to run which caused the "killed by server" error to appear in check50 even though it did not have an infinite loop and did not seg fault).

FIX: I simply hard coded my if block at the end of the program to only look for the end of jpg sequence in the last image and now I have all green smileys on check50. (I also had to add extra code to close the previous file before opening a new one because in the code above it closes the file when it finds the end of jpg sequence rather than right before opening the next file.)

As for resize.c, I still have no idea what the red TODOs are trying to tell me.