Exiting out of program with condition involving fread

Question

I am working on a photo extraction program for a course I am taking.

For this program, argv[1] is the name of the file from which I want to extract images from. jpg_name relates to the name of each image I am extracting; I am attempting to name each jpg. numerically starting from 1. As there are 50 photos to be extracted, I would like to stop the program after all 50 images have been extracted. Unfortunately, my program is not sure when to terminate, and as such I believe the last photo is being overwritten multiple times, and I am unsure how to fix this. However, photos 1-49 turn out fine, it is just photo 50 I am having issue with.

Things I have tried to far include implementing an if condition involving fread where if the program is unable to read 512 bytes of code into the array, it would terminate. Unfortunately, this does not seem to work either. Any suggestions would be appreciated:

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

int main(int argc, char *argv[])
{
    if (argc != 2)
    {
        printf("Invalid entry.\n");
        return 0;
    }

    int counter = 1;
    FILE* images;
    char jpg_name[8];

    // Check if bytes are jpg. signatures.
    for (int n = 0; counter < 51; n = n + 512)
    {
        // Open file for reading.
        FILE *file = fopen(argv[1], "r");
        if (!file)
        {
            return 1;
        }

        unsigned char array[512];
        fseek(file, n, SEEK_SET);
        fread(array, 1, 512, file); // if EOF, won't have 512 to write into!!!
        if (fread(array, 1, 512, file) != 512)
        {
            return 2;
        }

        if (array[0] == 0xff && array[1] == 0xd8 && array[2] == 0xff && (array[3] & 0xf0) == 0xe0)
        {
            // Convert integer to string and store into jpg character array. Increment image number.
            sprintf(jpg_name, "%03i.jpg", counter);
            counter++;

            // Open images file to write into, allocate memory to jpg file to write into, write 512 bytes from array into image file.
            images = fopen(jpg_name, "a");
            fwrite(array, 1, 512, images);
            fclose(images);
        }
        else // If 1st 4 bytes aren't jpg signature.
        {
            if (counter > 1)
            {
                images = fopen(jpg_name, "a");
                fwrite(array, 1, 512, images);
                fclose(images);
            }
        }
        fclose(file);
    }
}

Answer 1

You seem to have a basic misunderstanding about functions and return calls. I suspect you think: an fread instruction reads data, and an if instruction checks if something is true. So you read the data, then check how much was read. However, you are missing the fact that if executes the instructions inside the () in order to see whether the result is true. So if(fread(array, 1, 512, file) != 512) reads data and checks the result.

If you want to read data and check the result, use if(fread(array, 1, 512, file) != 512). If you want to read data and not check the result, use fread(array, 1, 512, file);. Don't use both.

You could also choose to use a variable, for example:

int number_of_bytes_read = fread(array, 1, 512, file);
if(number_of_bytes_read != 512) // this doesn't read data, it just gets the number from the variable
{
    ...

Answer 2

If I understand correctly, you are trying to extract images from one jpeg file and splitting it into multiple files. Here are a few questions to think about

1. fseek, why would you need that unless you are trying to go to the end of file for length and comeback to the current offset. Otherwise, I advice you to remove it.
2. Two freads are not required. Also, feof() should tell you if you have reached eof if fread gives anything other than what you are seeking for.

I have slightly edited your code, see if that is what you are looking for

    #include <stdio.h>
    #include <stdlib.h>
    #include <stdbool.h>
    #include <string.h>
    
    int main(int argc, char *argv[])
    {
        if (argc != 2)
        {
            printf("Invalid entry.\n");
            return 0;
        }
    
        int counter = 1;
        FILE* images;
        char jpg_name[8];
        int num = 0;
        int total_num = 0;
    
        // Check if bytes are jpg. signatures.
        for (int counter = 0; counter < 51; counter ++)
        {
            // Open file for reading.
            FILE *file = fopen(argv[1], "r");
            if (!file)
            {
                printf("Exiting at 1\n");
                return 1;
            }
    
            unsigned char array[512];
            // don't understand why you need this ? unless, you are trying to go to end of file and get back to the current
            // offset
            fseek(file, 0, SEEK_SET);
            total_num = 0;
            while((num = fread(array, 1, 512, file)) == 512)
            {
                if (array[0] == 0xff && array[1] == 0xd8 && array[2] == 0xff && (array[3] & 0xf0) == 0xe0)
                {
                    // Convert integer to string and store into jpg character array. Increment image number.
                    sprintf(jpg_name, "%03i.jpg", counter);
                    counter++;
    
                    // Open images file to write into, allocate memory to jpg file to write into, write 512 bytes from array into image file.
                    images = fopen(jpg_name, "a");
                    fwrite(array, 1, 512, images);
                    fclose(images);
                }
                else // If 1st 4 bytes aren't jpg signature.
                {
                    if (counter > 1)
                    {
                        images = fopen(jpg_name, "a");
                        fwrite(array, 1, 512, images);
                        fclose(images);
                    }
                }
            }
            fclose(file);
        }
        printf("Exiting here\n");
    }