0

I'm having trouble sussing out where I am having problems here, I'm pretty sure that I have covered everything that is mentioned in the walk-through.

Maybe I'm just missing something.

Stepping through my code I think that the files all close correctly and that I don't write to many bytes to a file, the file names seem to be correct.

I'm checking for all 16 of the fourth byte options of the starting string.

It's reading 512 bytes at a time, I have tried reading 512 bytes once and 1 byte 512 and writing the files both ways too. Now I'm trying to read 512 bytes and write only the size of the buffer (I think this would be determined by how many bytes were read?).

I think the while fread statement should read until it returns 0 (false) so should read all bytes in the raw file?

I'm aware that I don't have a clear, specific question, I just can't rationalise the problem.

Please can someone help me to work out where I am going wrong here.

Check50: https://sandbox.cs50.net/checks/73e5a8a00ab14d49b9a08257b32ba5bc

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

typedef uint8_t BYTE;

const int byteBlock = 512; // Set block size

bool compare(BYTE buffer[]); // declare compare function for later use.

int main()
{

    FILE* file = fopen("card.raw", "r");
    if (file == NULL)
    {
        printf("Could not open file.\n");
        return 1;
    }

    int count = 0; // Start counter for file names
    BYTE buffer[byteBlock]; // Set buffer size
    char outfile[8]; // Set output file name string length
    FILE* jpg = NULL; // Set JPG to NULL

    // Read 512 bytes until fead fails to read an bytes
    while (fread(&buffer, 1, byteBlock, file)) {
        // Check first four bytes
        if (compare(buffer)) {
            // If jpg has been set close previous jpg file
            // If NULL it's the first occurence so ignore
            if (jpg != NULL)
                fclose(jpg);
            // Set out file title (outfile 8 char not 7 due to char to end string).
            sprintf(outfile, "%03d.jpg", count);
            // Open new file
            jpg = fopen(outfile, "w");
            // If can't open file return error.
            if (jpg == NULL)
            {
                fprintf(stderr, "Could not create %s.\n", outfile);
                return 2;
            }
            // Increase count for next file name.
            count++;
        }
        // Start writing and continue writing once jpg has been found.
        if (jpg != NULL)
            fwrite(&buffer, 1, sizeof(buffer), jpg);
    }
    // Close last jpg file.
    fclose(jpg);
    // Close read file.
    fclose(file);
    return 0;
}

bool compare(BYTE buffer[])
{
    // First 3 bytes of jpeg
    const BYTE startSeq[] = {0xff, 0xd8, 0xff};
    // Options for fourth byte of jpeg
    const BYTE lastSeq[] = {0xe0, 0xe1, 0xe2, 0xe3, 0xe4, 0xe5, 0xe6, 0xe7, 0xe8, 0xe9, 0xea, 0xeb, 0xec, 0xed, 0xee, 0xef};

    // Compare first 3 bytes against startSeq
    for (int i = 0; i < 3; i++)
        if (buffer[i] !=  startSeq[i])
            return false;

    // Compare fourth byte against all lastSeq options
    for (int i = 0; i < 16; i++) {
        if (buffer[3] == lastSeq[i]) {
            // exit loop if fourth byte matches
            break;
        } else {
            return false;
        }
    }
    // First four bytes match jpeg start.
    return true;
}
1

The problem lies in the compare function. When it checks the 4th byte, it intends to compare the buffer to the 16 possible choices. Unfortunately, because of how it's written, it only compares the first of the 16 possibilities. On the first pass of the for loop, if it matches, the break executes, leaving the for loop and the return true executes. If it doesn't match, the return false executes. Either way, it never checks the second possibility.

The reason it works locally but check50 fails is that check50 uses a different test data set that exposes this problem.

The fix is to rewrite the for loop that checks the 4th byte.

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

3
  • Thanks so much, ha so obvious now. – OrderAndChaos Apr 10 '16 at 16:24
  • You know I spent all my time looking over and double checking all the stuff I didn't understand that I overlooked the simple stuff. Might have been handy if the test data provided picked up on this stuff too. – OrderAndChaos Apr 10 '16 at 16:38
  • Having said that, without this bug I wouldn't have understood fread and fwrite half as well as I do know! – OrderAndChaos Apr 10 '16 at 16:42

You must log in to answer this question.

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