0

The lines which attempt to stop the recursion using EOF don't seem to work. Can anyone help me see why?

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


//functions
void code(int block, FILE* in, FILE* out);
char* namer(int n);
char* name;


//variables;
int counter; //counts the number of NEW jpg NOT NUMBER OF BLOCKS
uint8_t buffer[512];

int main(int argc, char *argv[])
{
    if (argc != 2)
    {
        printf("Usage: /recover.c card.raw\n");
        return 1;
    }

    FILE *file = fopen(argv[1], "r");
        if (file == NULL)
        {
            return 1;
        }

    FILE* output = fopen("000", "w");
        if (file == NULL)
        {
            return 1;
        }

    code(0, file, output);

}

void code(int block, FILE* in, FILE* out)
{
    fseek(in, block, SEEK_SET); //resets file pointer to the first byte in file, then offsets to the appropriate block
    fread(buffer, 1, 512, in); //reads 1 block worth of stuff into buffer

    if (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && 224 <= buffer[3]) // if this is the first block of a new jpg
    {
        if (counter>0) //this is a 1st block, but not the first loop, i.e. maybe the 1st block of 2nd jpeg
        {
            fclose(out); // close previous file
            FILE* output = fopen(namer(counter), "w"); //open new file

            //write into new file
            fwrite(out, 512, 1, in);
            counter++;
            block += 512;
            code(block, in, out); //continue on to next block

            //first block of a jpg can also be last block of jpg, meaning it can be the last block in entire file so
            if (feof(in)!=0)
            {
                return;
            }
        }

        else //this is literally the first loop ever, meaning no need to close the previous file AND no need to name anything
        {
            //insert normal read and write code here
            fwrite(out, 512, 1, in);
            //counter - so that the next time they find a 1st block of a new jpg, the name will update
            counter++;
            //insert the call to open another fread
            block += 512;
            code(block, in, out);
        }
    }

    else // this is the 2nd, 3rd or nth block of a jpeg
    {
            //insert normal read and write code here
            fwrite(out, 512, 1, in);
            //insert the call to open another fread
            block += 512;
            code(block, in, out);

            //if this is the last block EVER
            if (feof(in)!=0)
            {
                return;
            }
    }


}


char* namer(int n)
{
    if (n < 10) //0 to 9
    {
        strcat(name, "00");
        sprintf(name, "%i", n);
        return name;
    }

    else if (10 <= n && n < 100) //10 to 99
    {
        strcat(name, "0");
        sprintf(name, "%i", n);
        return name;
    }

    else
    {
        sprintf(name, "%i", n);
        return name;
    }
}
1

I have had to look at this for a moment to get my head to understand the logic. You have written your feof condition in a way that should work, but it would be much easier to follow if written feof(in)==1 or feof(in) as both of these clearly evaluate to true; you currently have written a statement that reads "not false" which is just awkward to read, especially when trying to find out why something isn't working.

The problem that I see is that there is no return statement for the code methods, only for the if statements that check for the EOF. The other statements need a return statement after they call the code method so that they can return (or "walk" back up the recursion) to the original call and then the original method that called the code method. This means that the whole code method also needs a return statements. Right now, it looks as though this will hang, or sit forever, as the program will run out of things to execute without reaching the end of the program.

It's possible there might be something else, but everything else looks like reasonable logic to me and I don't see anything else that would cause you to say that "using EOF doesn't seem to work."

Edit: Ok, seeing that you are getting an check error of all routes call itself, I will go back to my original thought regarding your recursive function--it's base case is not properly defined. Upon rereading your code a few times, it seemed like maybe its structure wasn't the issue and you were saying it was just never finding the end of the file. But, now that we know that this isn't the case, let's look at how to declare the cases of the recursive function a little clearer for both the human reader and the computer.

What I see in your code method, currently, is a lot of linear logic, where the code is meant to run through nested if-else statements, narrowing down the possible cases until it finds a correct one. Your recursive function is almost identical to how this would be done without recursion. So, with restructuring your code to look for cases first, and not running into nested statements to find them, your code method should look something like this:

void code(FILE* in, FILE* out)
{
    fread(buffer, 1, 512, in);

    //base case; eof reached
    if(feof(in))
        return 0;

    //header found
    else if(buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == oxff && (buffer[3] & 0xf0) == 0xe0)
    {
        if(count > 0) //new JPEG header found
        {
            //code to close old file and open a new one
        }
    
        //code to write the buffer to the file and increment counter
        //this can stay out of the if because it always applies

        code(in, out);
        return 0;
    }

    //buffer contains contents of a JPEG
    else if (counter > 1)
    {
        //code to write buffer to file

        code(in, out);
        return 0;
    }

    //close the out file as the writing is done and exit the method
    fclose(out);
    return 0;
}

Now, I'll admit that this code is written off the top of my head and I haven't had the time to test it, but you should see how it's structure is different from what you have written. Personally, I would not do this problem recursively (you can almost copy-paste your recursive method into your main function, with few modifications and removing the recursive calls, and it would work), but this will hopefully help you get closer to finding a recursive solution to this. Obviously, there are many ways to write recursive methods (and all methods), but, in my experience, recursive methods are usually written with a structure that is different from non-recursive methods. I will admit, though, that the projects I work on do not often lend themselves to recursive logic (they are event-based), and so I haven't dealt with this much outside of coursework.

Hopefully this helps and let me know in the comments if you still have questions about this.

Edit 2: Let me try to explain how (buffer[3] & 0xf0) == 0xe0) works.

First, the last zero that was originally in that code snippet (you asked about (buffer[3] & 0xf0) == 0xe00) should have been a closing parenthesis. The correct code snippet, as I have listed at the start of this edit and corrected in my code example, is (buffer[3] & 0xf0) ==0xe0).

Now, we can discuss how the code works. This code is, or at least was, provided in the video that is in the problem set--I recommend watching them, even if you already have a working solution, as they introduce you to cool concepts like these that might be helpful sometime. The & operator in C is a bitwise operator, meaning that it operates on the provided values at their binary level, not their human-legible one. Specifically, this one compares the binary values of each bit, similar to how a Boolean operator would compare two values.

Then, instead of returning a simple true or false if the whole value provided is equal, it provides a value back whose binary is equal to which bits where present in both values. Remember that a bit of 0 is empty/off and a bit of 1 is filled/on. This can get a bit confusing to try and read (it is working at the computer's level of understand after all), so let's work through two examples:

bool true = (0xef & 0xf0) == 0xe0;
bool false = (0xff & 0xf0) == 0xe0;

Ultimately, we are using this bitwise operator to perform a boolean comparison, so I have set the examples to fill to boolean variables (as they will get resolved down into a regular true/false quantity in order to fulfill the if statement's condition requirements). These examples as written don't really help with anything, right? I mean, they are still in a human-legible format. So, let's translate the parts being compared into binary:

bool true = (11101111 & 11110000) == 11100000;
bool false = (11111111 & 11110000) == 11100000;

Ok, now we are closer to what the computer will be reading and can really dissect this. The & operator will return a 1 where both values have a 1 and a 0 otherwise. So, if you go bit-by-bit through the first example, the expression in the parentheses becomes (11100000). This is equal to 11100000 (0xe0), so the whole expression is true. For the second one, though, the expression in the parentheses becomes (11110000). Because this is not equal to 11100000, the whole expression evaluates to false.

In the problem set, we are told that 0xe0, 0xe1, 0xe2, 0xe3, 0xe4, 0xe5, 0xe6, 0xe7, 0xe8, 0xe9, 0xea, 0xeb, 0xec, 0xed, 0xee, and 0xef all have 1110 as their first four bytes. So, when plugged into the expression (buffer[3] & 0xf0) == 0xe0, they will all evaluate to true just as 0xef did above. For me, this seemed like the cleanest and most succinct way to deal with the great variety that the fourth byte of the header could have.

Let me know if any of that doesn't make sense.

5
  • thank you so much for taking the time to read through my code! The reason i wrote feof condition in that way was that I thought the output feof would return was 0 in the case of not true and any non-zero number in the case of true. I have added a "return" after each method and in the larger code method. Testing this still led to an "all paths through this function will call itself" error, so I put the eof condition check before the call to the recursive function.
    – Melvin
    Jan 28 at 9:16
  • That error returns my thinking to where it was initially about the recursive method needing to be restructured to use better recursive logic. I have edited my answer to include this and hopefully it will help. Let me know if something doesn't make sense. Jan 28 at 12:38
  • Hi thank you once again for even spelling it out for me! Could I ask how the below code works? (buffer[3] & 0xf0) == 0xe00
    – Melvin
    Jan 29 at 7:28
  • Not a problem. That code should read (buffer[3] & 0xfe0) == 0xe0). I fixed it in my example and explained how it works in another edit to my answer (it would be too long to post in a comment). Let me know if anything doesn't make sense and don't forget to accept the answer if it's helped solve your problem so the system knows this is resolved (and doesn't keep sending this to the top of the New feed). Jan 29 at 12:10
  • Let me also add that the code (buffer[3] & 0xfe) == 0xe0 (yes, I typed it wrong, again, in the comment above) was all that was provided by the video. The rest, as is often true, required me researching the new concept I saw and learning how it worked. But, without seeing it in the video, I never would have known to look it up. :) Jan 29 at 12:42

You must log in to answer this question.

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