Why can't I assign the return value of a fwrite() function to another variable? Infinite loop

Question

In 'recover' I wanted to jump out of my while loop if the fwrite() function of the last photo (of the file) returns a value < 512 (indicating that the last photo or end of file is reached). I wanted to assign it to an int variable called 'status'. But somehow in debug50 it seems like the variable doesnt get assigned the return value properly and so my while loop runs indefinitely.

Any thoughts on how I could implement this without writing the whole code all over again?

If there are any other thoughts on my code, hit me. Want to learn as much as I can.

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

int main(int argc, char *argv[])
{
    //argv commands: if not one argument after prog: "Usage: ./recover image", return 1
    if (argc != 2)
    {
        printf("Usage: ./recover image\n");
        return 1;
    }

    //open memory card; if opening not possible print and return 1
    char *input = argv[1];

    FILE *inputptr = fopen(input, "r");
    if (input == 0)
    {
        printf("File could not be opened.\n");
        return 1;
    }

    typedef uint8_t byte;
    byte buffer[512];
    char *filename = malloc(8);
    bool first_found = false;
    int counter = 0;
    bool eof = false;
    int status = 513;
    FILE *outputptr = NULL;

    //Repeat until end of card
    while(status >= 512) //until end of card
    {
        //read 512bytes into a buffer and watch for jpeg start bytes
        fread(buffer, sizeof(byte), 512, inputptr);


        //if start of new jpeg
        if (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && ((buffer[3] & 0xf0) == 0xe0))
        {

            //else: every other after first we have to close previous file
            if (first_found == true)
            {
                counter++;
                fclose(outputptr);
                sprintf(filename, "%03i.jpg", counter);
                outputptr = fopen(filename, "w");
                fwrite(buffer, sizeof(byte), 512, outputptr);
            }
            //if first jpeg
            if (first_found == false)
            {
                first_found = true;
                sprintf(filename, "%03i.jpg", counter);
                outputptr = fopen(filename, "w");
                fwrite(buffer, sizeof(byte), 512, outputptr);
            }
        }
        //else: writing in already open jpeg
        else if (first_found == true)
        {
            status = fwrite(buffer, sizeof(byte), 512, outputptr);
        }
    }

    //close reamining files, free() memory
    fclose(inputptr);
    fclose(outputptr);
    printf("Successful, goodbye.\n");
    free(&filename);
    return 0;
}

Answer 1

I think you've misunderstood what's happening. In reality, status is being assigned 512 on every pass.

There's a flaw in your logic. It's looking at the fwrite when it should look at fread. The loop is executing an fread at the beginning of each pass. When it gets to the end of the input file, it attempts the fread, but reads nothing. Since it reads nothing, buffer remains unchanged. Later, the fwrite calls write out the contents of fwrite successfully, and assigns 512 to status. In short, it goes into an infinite loop, reading nothing and writing the same last 512 bytes out to the last output file.

You could use status to store the return value from the fread, but that still creates a problem. Unless you check that value immediately after the fread and break out of the loop, it will execute one extra write on the last output file (writing the last block a second time), before returning to the top of the loop and seeing that status is 0.

If you want to understand it better, run debug50 and see what happens to status after the first fwrite. You could also create a variable to store the return code from the fread to see what it does. You could also fprint("status = %i \n",status); at strategic points.

Maybe you could incorporate the fread into the while statement and check it's return code there? ;-)

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