0

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;       
}
1
  • I just realized I am getting the same exact "TODO" error for my resize.c program which I had given up on even though it seemed to work correctly (sandbox.cs50.net/checks/d2593b5a14654b9da65068f3ab812345). I created the 1x1 and 2x2 pixel bmp files and tested like check50 did and each one seemed to resize correctly for me. I am at a complete loss of what to do. Thanks again for any help! – user9530 Sep 11 '15 at 0:39
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.

5
  • I apologize if my code is confusing...I am not a programmer, but am trying to learn. This is what I believe my code is doing: – user9530 Sep 11 '15 at 13:24
  • (1) open card.raw and until I reach the end of card.raw do the following: (2) read in 512 bytes from card.raw (3) look for start of jpg file (3a) if found open and name new file and write the 512 bytes to it (3b) if start of jpg is not found and there is file still open, write to it (4) if there is file open check the current 512 bytes for jpg end of file sequence (4a) if end of jpg sequence is found close file and set it to NULL (hence 3b will writes only when it should). After this, I close card.raw. – user9530 Sep 11 '15 at 13:34
  • As for the for loop you mention doesn't do anything useful, it is looking for the end of each jpg. I realize that since the jpg are stored on the card back to back, I could just look for this in the last jpg, but I was not sure how to do this without hardcoding in the number of images to expect. If I let the program just add the entire end of card.raw to the last image then the final image although it opens is 1.2 GB which seems ridiculous. – user9530 Sep 11 '15 at 13:38
  • As for the (feof(file) == 0), does anyone have a better idea? I was trying to avoid hardcoding in the expected number of images, which is the only other way I know of to stop the loop. – user9530 Sep 11 '15 at 13:39
  • 1
    There is no such thing as "end of jpeg". At least not as far as we can care about. When a new jpeg starts, by logic, it means that the previous one has ended. I did misread the bytes you were comparing to and that's why i misunderstood. But there's no reason to check for the end of a jpeg. Look into what fread returns, and think of how you can use that to check whether or not the end of file has been reached – Irene Sep 11 '15 at 21:52
0

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.

You must log in to answer this question.