1

my first post and would appreciate some feedback. I'm taking CS50 online, and the course assumes a lot, especially from someone like me who's only taking up coding again after 15 years (think Basic & Turbo Pascal when the Internet was still young...)

In any case, I've read and researched regarding my code below, with plenty of comparing between my (non-working) attempt and other (working) solutions found via Google etc. I need a eye with more training/experience to tell me why this code produces 50 JPEGs that are 98% empty (grey space) with a few lines of weird coloring at the top left.

The assignment is of course to extract 50 JPEG images from a RAW file with data by identifying the JPEG headers and writing them into 50 new files. My attempt follows...

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

int main(void)
{
FILE* outptr = NULL;

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

unsigned char buffer[512] = {0};
int jpgcounter = 0;
char jpgname[8];

// FIND BEGINNING OF NEW JPEG
while(fread(buffer, 512, 1, file) == 1)
{
    if ((buffer[0] = 0xff) && (buffer[1] == 0xd8) && (buffer[2] == 0xff) && ((buffer[3] == 0xe0) || (buffer[3] == 0xe1)))
    {

        // CLOSE OLD FILE IF OPEN
        if (outptr != NULL)
        {
            fclose(outptr);
            outptr = NULL;
        }

       // CREATE NEW JPG NAME
        sprintf(jpgname, "%03d.jpg", jpgcounter);
        jpgcounter++;

        // OPEN NEW JPG
        outptr = fopen(jpgname, "w");   
        if (outptr == NULL)
        {
            printf("Could not create JPG!\n");
            return 2;
        }

        // WRITE FIRST BLOCK OF IMAGE
        fwrite(buffer, sizeof(buffer), 1, outptr);

    // WRITE ADDITIONAL LINES OF IMAGE
    }
    else
    {
        if(NULL!=outptr)
        fwrite(buffer, sizeof(buffer), 1, outptr); 

    }   
}
fclose(outptr);
fclose(file);
return 0;

}

4
  • I'm working on recover.c. Very close to finishing and trying to look at others' code to understand how I can get mine finished. Can you explain to me this line of code: while(fread(buffer, 512, 1, file) == 1) Can you explain the "==1" part? – Scott Jul 19 '16 at 0:25
  • Scott, the way I understand it is that fread returns 1 when the command in the brackets is executable (in other words as long as there are blocks of 512 bytes to read). The loop will thus terminate when the end of the file is reached and fread returns something else than 1. – Leon Jul 19 '16 at 7:57
  • Excellent, thank you – Scott Jul 19 '16 at 16:16
  • @Leon Thank you very much. – toyotasupra Sep 11 '17 at 12:51
2

your program works perfectly, congratulations, there is only a "small error"

(buffer[0] = 0xff)

it should be

(buffer[0] ==0xff)

1
  • Fresh eyes! Thanks! – Leon Jul 17 '16 at 18:43
1

I just finished the PSET myself but was browsing here because I was struggling with a segmentation error. Anyway, noticed that our structure of the code is pretty much identical. I only noticed one difference at the end your code that I found interesting.

Just a friendly suggestion for optimization. You could lose some additional lines and duplicate code, if you change your code to not have separate fwrite statements for the 1st time and consecutive times of looping through a jpg.

Since your if(NULL!=outptr) condition will off course also return TRUE when you just created a new jpg, you could lose one of the fwrite's and the else condition per below:

...
// OPEN NEW JPG
    outptr = fopen(jpgname, "w");   
    if (outptr == NULL)
    {
        printf("Could not create JPG!\n");
        return 2;
    }
}

// WRITE BUFFER TO OUTPUTFILE
if(NULL!=outptr)
{
    fwrite(buffer, sizeof(buffer), 1, outptr); 
} 

Again, just meant as a friendly suggestion. Not trying to be a know it all :).

You must log in to answer this question.

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