Pset4 recover, comparing hex in if statements

Question

I'm trying to solve "recover" but, I'm unable to compare my if statements. This is my error message:

error: comparison of constant 224 with expression of type 'char' is always false [-Werror,-Wtautological-constant-out-of-range-compare] if(store[3] == 0xe0 || store[3] == 0xe1 || store[3] == 0xe2 || store[3] == 0xe3 || store[3] == 0xe4 || store[3] == 0xe5 || store[3...

Should I be changing my store[i] to char.. or the hex to int.. How do I compare them..?

In Pset4/resize, they used structs and was able to access each byte, how can i do the same without using structs..?

/**
 * recover.c
 *
 * Computer Science 50
 * Problem Set 4
 *
 * Recovers JPEGs from a forensic image.
 */

#include <stdio.h>
#define MEMORYBLOCK 512
#include <cs50.h>

int main(void)
{
    //declare variables
    char memory[MEMORYBLOCK];       //int == 4 bytes, we want, 1 byte 
    char store[4];

    //chars of arrays for sprintf
    char title[sizeof "###.jpg"];

    //could the 4th byte belong to a jpg?
    bool fourthByteJpg = false;

    //filename counter
    int fileNameCounter = 0;

    //found jpg?
    bool foundJpg = false;

    //firstPass = true
    bool firstPass = true;

    bool foundFirstThreeBytes = false;
    bool storeIsJpg = false;
    //MY PSEUDOCODE






//solution 3 (I think this is it..!!)
//once open
   FILE* inptr = fopen("card.raw", "r");
    if(inptr == NULL)
    {
        printf("could not open file\n");
        fclose(inptr);
        return 1;
    }

    //size of card
    fseek(inptr, 0, SEEK_END);
    int cardSize = ftell(inptr);

        //for each block in the card
        for(int h = 0, noOfBlocks = cardSize/512; h < noOfBlocks; h++)
        {
            //for within each 512 memory block
            for(int i = 0; i < MEMORYBLOCK; i++)
            {
                //read & store 512bytes in memory[i]
                fread(&(memory[i]), 1, 1, inptr);


                //read & store first four bytes into store[i]
                if(i < 4)
                {
                    fread(&(store[i]), 1, 1, inptr);
                }
            }
            //define if store is jpg
            if(store[3] == 0xe0 || store[3] == 0xe1 || store[3] == 0xe2 || store[3] == 0xe3 || store[3] == 0xe4 || store[3] == 0xe5 || store[3] == 0xe6 ||
            store[3] == 0xe7 || store[3] == 0xe8 || store[3] == 0xe8 || store[3] == 0xe9 || store[3] == 0xea || store[3] == 0xeb || store[3] == 0xec || store[3] == 0xed
             || store[3] == 0xee || store[3] == 0xef)
             {
                 fourthByteJpg = true;
             }
             if(store[0] == 0xff && store[1] == 0xd8 && store[2] == 0xff)
             {
                 foundFirstThreeBytes = true;
             }

            if((foundFirstThreeBytes == true) && (fourthByteJpg == true))
            {
                storeIsJpg = true;
            }

            //if store[1],[2],[3],[4] is jpg && already found jpg
            if((storeIsJpg == true) && (foundJpg == true))
            {
                //fileNameCounter ++
                fileNameCounter++;
                //foundJpg = false
                foundJpg = false;
                //create a new file, name it accordingly
                sprintf(title, "%03d.txt", fileNameCounter);
                //open the newly created file
                FILE* outptr = fopen(title, "w");
                if(outptr == NULL)
                {
                    printf("could not create file\n");
                    fclose(outptr);
                    return 2;
                }

            }
            //if store[1],[2],[3],[4] is jpg || already found jpg
            if((storeIsJpg == true) || (foundJpg == true))
            {    //foundJpg = true
                foundJpg = true;
                //if(firstPass = true)
                    //open the newly created file
                    FILE* outptr = fopen(title, "w");
                    if(outptr == NULL)
                    {
                        printf("could not create file\n");
                        fclose(outptr);
                        return 2;
                    }

                if(firstPass == true);
                {
                    //create a new file, name it accordingly                     //fopen("fileNameCounter.jpg", "w")
                    //create a new file, name it accordingly
                    sprintf(title, "%03d.txt", fileNameCounter);

                    //firsPass = false
                    firstPass = false;
                }   
                //write current memory[i] into output                   ////fwrite(&(store[i]), 1, 1, outptr);
                 for(int i = 0; i < MEMORYBLOCK; i++)
                {
                    fwrite(&(memory[i]), 1, 1, outptr);
                }
            }   
        }


    //remember to close opened files and free memory(if any)
    fclose(inptr);
}

Version 2:

I added the functions as Matt Obert has suggested and still, it isn't getting the jpgs out..

/**
 * recover.c
 *
 * Computer Science 50
 * Problem Set 4
 *
 * Recovers JPEGs from a forensic image.
 */

#include <stdio.h>
#define MEMORYBLOCK 512
#include <cs50.h>
#include "jpg.h"

bool jpgCheck(BYTE data[], BYTE jpgSig[]);

int main(void)
{
    //declare variables
    BYTE store[MEMORYBLOCK];       //int == 4 bytes, we want, 1 byte 

    //chars of arrays for sprintf
    char title[sizeof "###.jpg"];


    //filename counter
    int fileNameCounter = 0;

    //found jpg?
    bool foundJpg = false;

    //firstPass = true
    bool firstPass = true;

    bool storeIsJpg = false;


    // JPEG signature
    BYTE jpgSig[3] = { 0xff, 0xd8, 0xff };


    //MY PSEUDOCODE


//solution 3 (I think this is it..!!.. [update: 7-Dec-2016] just kidding.. not it.)
//once open
   FILE* inptr = fopen("card.raw", "r");
    if(inptr == NULL)
    {
        printf("could not open file\n");
        fclose(inptr);
        return 1;
    }

    //size of card
    fseek(inptr, 0, SEEK_END);
    int cardSize = ftell(inptr);

        //for each block in the card
        for(int h = 0, noOfBlocks = cardSize/512; h < noOfBlocks; h++)
        {
            //for within each 512 memory block
            for(int i = 0; i < MEMORYBLOCK; i++)
            {
                //read & store 512bytes in memory[i]
                fread(&(store[i]), 1, 1, inptr);

            }

            if(jpgCheck(store, jpgSig) == true)
            {
                storeIsJpg = true;
            }


            //if store[1],[2],[3],[4] is jpg && already found jpg
            if((storeIsJpg == true) && (foundJpg == true))
            {
                //fileNameCounter ++
                fileNameCounter++;
                //foundJpg = false
                foundJpg = false;
                //create a new file, name it accordingly
                sprintf(title, "%03d.txt", fileNameCounter);
                //open the newly created file
                FILE* outptr = fopen(title, "w");
                if(outptr == NULL)
                {
                    printf("could not create file\n");
                    fclose(outptr);
                    return 2;
                }

            }
            //if store[1],[2],[3],[4] is jpg || already found jpg
            if((storeIsJpg == true) || (foundJpg == true))
            {    //foundJpg = true
                foundJpg = true;
                //if(firstPass = true)
                    //open the newly created file
                    FILE* outptr = fopen(title, "w");
                    if(outptr == NULL)
                    {
                        printf("could not create file\n");
                        fclose(outptr);
                        return 2;
                    }

                if(firstPass == true);
                {
                    //create a new file, name it accordingly                     //fopen("fileNameCounter.jpg", "w")
                    //create a new file, name it accordingly
                    sprintf(title, "%03d.txt", fileNameCounter);

                    //firsPass = false
                    firstPass = false;
                }   
                //write current memory[i] into output                   ////fwrite(&(store[i]), 1, 1, outptr);
                 for(int i = 0; i < MEMORYBLOCK; i++)
                {
                    fwrite(&(store[i]), 1, 1, outptr);
                }
            }   
        }


    //remember to close opened files and free memory(if any)
    fclose(inptr);
}

//function to JpgCheck
bool jpgCheck(BYTE data[], BYTE jpgSig[])
{
    for (int i = 0; i < 3; i++)
    {
        if ((data[i]) != (jpgSig[i]))
        {
            return false;
        }
    }

    if (data[3] != 0xe0 || data[3] != 0xe0 || data[3] != 0xe1 || data[3] != 0xe2 || data[3] != 0xe3 || data[3] != 0xe4 || data[3] != 0xe5
     || data[3] != 0xe6 || data[3] != 0xe7 || data[3] != 0xe8 || data[3] != 0xe8 || data[3] != 0xe9 || data[3] != 0xea || data[3] != 0xeb
      || data[3] != 0xec || data[3] != 0xed || data[3] != 0xee || data[3] != 0xef)
    {
        return false;
    }
    return true;
}

The jpg.h file

#include <stdint.h>
typedef uint8_t BYTE;

Below is my pseudo code to ease the reading (if required):

//solution 3 
//once open
    //firstPass = true
        //for each block in the card                        //for(int h = 0, noOfBlocks = cardSize/512; h < noOfBlocks; h++)
            //for within each 512 memory block                        //for(int i = 0; i < MEMORYBLOCK; i++)
                //read & store 512bytes in store[i]         //fread(&(store[i]), 1, 1, inptr);

            //if this 512 block is jpg && already found jpg
                //fileNameCounter ++
                //foundJpg = false
                //create a new file, name it accordingly                     //fopen("fileNameCounter.jpg", "w")

            //if this 512 block is jpg  || already found jpg
                //foundJpg = true

                //if(firstPass = true)
                    //create a new file, name it accordingly                     //fopen("fileNameCounter.jpg", "w")
                    //firsPass = false

                    //write current memory[i] into output                   ////fwrite(&(store[i]), 1, 1, outptr);

//close files

Answer 1

@Blauelf is correct: you can't count on char to be an unsigned 8-bit int.

However, you can use a typedef to make certain that a BYTE is really a byte.

Look at these type definitions, borrowed from bmp.h:

#include <stdint.h>

/**
 * Common Data Types 
 *
 * The data types in this section are essentially aliases for C/C++ 
 * primitive data types.
 *
 * Adapted from http://msdn.microsoft.com/en-us/library/cc230309.aspx.
 * See http://en.wikipedia.org/wiki/Stdint.h for more on stdint.h.
 */
typedef uint8_t  BYTE;   // You can use this one in recover.c
typedef uint32_t DWORD;
typedef int32_t  LONG;
typedef uint16_t WORD;

Note: I just looked back at your other question and saw that you are already using this in your jpg.h file. It would be sufficient to include the typedef directly in recover.c and avoid defining another struct here.

In other news, you're still over-complicating your approach. For example, if you are reading your data stream into a 512-byte buffer, you do not need to copy the first four bytes of your buffer into another array before comparing them to your JPEG signature. You can just compare the first four bytes of your buffer to the JPEG signature directly.

There's more than one way to do this. Here's one way to approach it using an array:

// JPEG signature
BYTE jpgSig[3] = { 0xff, 0xd8, 0xff };

Then, you could write a function to check the first three bytes like so:

bool jpgCheck(BYTE data[], BYTE jpgSig[])
{
    for (int i = 0; i < 3; i++)
    {
        if ((data[i]) != (jpgSig[i]))
        {
            return false;
        }
    }
    return true;
}

This is an incomplete solution (it still does not check for all the possible values of the fourth byte) but it should be enough to get you started.

Don't forget to accept the most helpful answer to each of your questions by clicking the green check mark; otherwise they will always be marked as unanswered questions, and they will clutter the forum. Fight Zombie Questions!

Answer 2

char can be signed or unsigned by default (and is 8-bit on every system I know, but that's not guaranteed by the specs).

On this system, char datatype is signed 8-bit int, ranging from -128 to 127, and 224 is outside that range. You can use unsigned char or BYTE instead, which ranges from 0 to 255.