0

Hi I've been working on recover for about a day now and I'v come up with some code that works logically to me but every time I try to run it the CPU usage that is indicated by the bar graph in the top right of the CS50 IDE goes through the roof and the program doesn't return anything neither does art exit the program. Could someone please tell me what's wrong?

P.S I'm aware that I haven't implemented the opening of different named JPEG files I wanted to make sure the program runs smoothly.

/**
 * recover.c
 *
 * Computer Science 50
 * Problem Set 4
 *
 * Recovers JPEGs from a forensic image.
 */
 #include <cs50.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <ctype.h>
 #include <stdint.h>



int main(int argc, char* argv[])
{
    //Opening the raw file
    FILE* inputfile = fopen("card.raw", "r");
    //Confirming that the file was opened correctly
    if(inputfile == NULL)
    {
        printf("Could not open card.raw\n");
        return 1;
    }
    // Going through the raw file until a JPEG is identified and the draw function is called and the image is written

    uint8_t header;
    uint8_t headertwo;
    uint8_t headerthree;
    uint8_t headerfour;


    do
    {

        void* mem = malloc(508);
        fread(&header, 1, 1, inputfile);
        fread(&headertwo, 1, 1, inputfile);
        fread(&headerthree, 1, 1, inputfile);
        fread(&headerfour, 1, 1, inputfile);
        fread(mem, 508, 1, inputfile);
        if(header == 0xff && headertwo == 0xd8 && headerthree == 0xff && (headerfour == 0xe0 || headerfour == 0xe1 || headerfour == 0xe2 || headerfour == 0xe3 || headerfour == 0xe4 || headerfour == 0xe5 || headerfour == 0xe6 || headerfour == 0xe7 || headerfour == 0xe8 ||headerfour ==  0xe9 || headerfour == 0xea || headerfour == 0xeb || headerfour == 0xec || headerfour == 0xed || headerfour == 0xee || headerfour == 0xef))
        {
            FILE* output = fopen("jpg.jpg", "w");
            fwrite(&header, 1, 1, output);
            fwrite(&headertwo, 1, 1, output);
            fwrite(&headerthree, 1, 1, output);
            fwrite(&headerfour, 1, 1, output);
            fwrite(mem, 508, 1, output);
            do
            {
                void* rmem = malloc(512);
                fread(rmem, 512, 1, inputfile);
                fwrite(rmem, 512, 1, output);
                free(rmem);
                //Looking ahead to make sure th next 512 byte chunk isnt the start of a new JPG
                fread(&header, 1, 1, inputfile);
                fread(&headertwo, 1, 1, inputfile);
                fread(&headerthree, 1, 1, inputfile);
                fread(&headerfour, 1, 1, inputfile);
                //Moving the cursor four bytes back as we just read four bytes ahead
                fseek(inputfile, -4, SEEK_CUR);



            }
            while(header != 0xff && headertwo != 0xd8 && headerthree != 0xff && (headerfour != 0xe0 || headerfour != 0xe1 || headerfour != 0xe2 || headerfour != 0xe3 || headerfour != 0xe4 || headerfour != 0xe5 || headerfour != 0xe6 || headerfour != 0xe7 || headerfour != 0xe8 ||headerfour !=  0xe9 || headerfour != 0xea || headerfour != 0xeb || headerfour != 0xec || headerfour != 0xed || headerfour != 0xee || headerfour != 0xef));
            fclose(output);
        }
        else
        {
            fseek(inputfile, 512, SEEK_CUR);
        }


        free(mem);
    }
    while(feof(inputfile) != true);


    fclose(inputfile);

}
0

There are several issues with this code. To be honest, I didn't work through all of them because of the severity of some and the need to redesign, which will change some of the minor issues. So, here's what I found.

In the single else clause, you have this line:

        fseek(inputfile, 512, SEEK_CUR);

This is going to skip over a lot of data that will never be processed. It's also causing signatures to be skipped.

Second, the last while test clause in the program is trying to check that the beginning of the code block is not a signature. Unfortunately, you can't implement it this way. Applying the not operator ! to each subclause of the test will not do what you're trying to do. (It's a semester long class in digital logic to explain all the reasons, or you can investigate it on your own.) The way to do this is to take the positive test that exists earlier in the code, put parentheses around it and put the ! sign before it. For example, if the test were A && B && ( C || D ) then the easy way to check for NOT that would be !( A && B && ( C || D ) )

You could also simplify the code a lot with some redesign. Instead of reading one byte each into 4 different vars plus the next 508 into another buffer, you could read 512 bytes into one single buffer array of type uint8_t. You could then check each of the first 4 bytes using array notation like buffer[0]. It simplifies the coding and eliminates 4 of 5 reads. (Disk reads are highly inefficient compared to memory reads by about a factor of 1000.) Also, you could allocate the buffer once at the top of the program and reuse it instead of allocating it and freeing it repeatedly.

This will get you started, but there are more issues remaining. I'll let you have the chance to work through it. If you have additional questions, you can always post a new question.

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

You must log in to answer this question.

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