Home > other >  CS50 PSET4 RECOVER: fread() not populating buffer array
CS50 PSET4 RECOVER: fread() not populating buffer array

Time:04-02

Hello and thank you for taking a look.

I'm working through CS50x and am struggling with Recover. The aim is to open a .raw file, read its contents in 512-byte blocks, check the initial four bytes for .jpg headers, and then write each JPEG data to a new file.

I have a body of code written, and the file compiles. The debugger tells me that my buffer[512] variable remains empty/zeroed. This then means the program skips if/else conditions and the program exits.

While my logic within the While loop may be flawed, I haven't been able to step far enough into the program to consider this.

I looked up my issue before posting. Some sources like to use fread(buffer, 512, 1, input), but CS50 itself uses fread(buffer, 1, 512, input). Also, when initialising the filename, I have tried both char *filename = malloc(8 * sizeof(char)); and char filename[8];. For both lines I have tried each method and am still missing something.

My code is below. Thank you in advance for your time.

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

typedef uint8_t BYTE;

int main(int argc, char *argv[])
{
    // First check the number of arguments is correct.
    if (argc != 2)
    {
        printf("Correct Usage: ./recover.c [filename]\n");
        return 1;
    }

    // Open the file.
    FILE *inputFile = fopen(argv[1], "r");

    if (inputFile == NULL)
    {
        printf("File not found.\n");
        return 1;
    }

    // Create counter of number of files.
    int counter = 0;
    // Create filename variable
    char *filename = malloc(8 * sizeof(char)); // 7   1 for \0
    // Create a 512-size array buffer.
    BYTE buffer[512];
    // Initialise img file for scope access.
    FILE *img = NULL;

    while (fread(buffer, sizeof(BYTE), 512, inputFile))
    {
        // If start of new JPEG:
        if (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[3] & 0xf0) == 0xe0)
        {
            if (counter == 0) // If the FIRST JPEG
            {
                // Make new file:
                sprintf(filename, "i.jpg", counter);
                img = fopen(filename, "w");
                fwrite(buffer, sizeof(BYTE), 512, img);
            }
            else // If not the first JPEG
            {
                fclose(img); // Close previous file.
                counter  ;
                // Make new file:
                sprintf(filename, "i.jpg", counter); // Update filename.
                img = fopen(filename, "w");
                fwrite(buffer, sizeof(BYTE), 512, img);
            }
        }
        else if (counter > 0) // buffer is continuation of previous.
        {
            fwrite(buffer, sizeof(BYTE), 512, img);
        }
        else
        {
            printf("I exited with no images.\n");
            return 2;
        }
    }
    free(filename);
    fclose(img);
    fclose(inputFile);
    return 0;
}

CodePudding user response:

The program exits (returns) after the first line in the raw file is read (assuming it's not a jpeg header, which is the case with the distro raw file). else if (counter > 0) evaluates to false, so the else branch executes.

CodePudding user response:

Thank you everyone for your response. The issue is now fixed!

@DinoCoderSaurus (sorry, can't upvote yet) prompted me to realise that I had assumed (wrongly) that the data in the raw file would immediately begin with a .jpeg header (in fact it looks like the data begins with a hidden message, "surprise").

The Else condition was initially put there to avoid errors but of course it was prematurely exiting the While loop. The buffer was populated correctly after a couple of loops.

I then encountered the second problem (pointed out by @Some_programmer_dude) that counter ; was in the wrong place, which meant after the first new JPEG, no others could be written.

I'll also take your comments about best practice into consideration.

  • Related