Welcome fellow students, register yourself and dive into asking questions, giving answers and helping the community learn with you.

recover.c check50 error

0 votes
asked Jun 17, 2016 in Pset4 by storm (290 points)
This is my code for recover.c . It successfully recovers to all jpg from 000.jpg 049.jpg .

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

int main(int argc, char* argv[])
{
    // TODO
    
    FILE*infile = fopen("card.raw", "r");
    char buf[512] = {0};
    char jpeg[4] = {0xff, 0xd8, 0xff, 0xe0};
    char jpeg1[4] = {0xff, 0xd8, 0xff, 0xe1};
    char filename[30];
    int filenumber = 0;
    int c = 0;
    FILE* outfile;
    
    while(c != EOF)
    {
       for (int i = 0; i < 512; i++)
       {
            buf[i] = c = fgetc(infile);
        }
       
       if(memcmp(buf, jpeg, 4) == 0 || memcmp(buf, jpeg1, 4) == 0)
       {
           if(outfile)
           fclose(outfile);
           
           int n;
           n= sprintf(filename, "%03d.jpg", filenumber);
           filenumber++;
           
           outfile = fopen(filename, "w");
           if(!outfile)
           {
               printf("error opening outfile\n");
               return 1;
           }
           fwrite(buf, 1, 512, outfile);
       }
       else if(outfile)
       {
            fwrite(buf, 1, 512, outfile);
       }
    }
    fclose(infile);
    if(outfile)
    fclose(outfile);
   
    return 0;
    
}

but check 50 is giving this error

storm12345@ide50:~/workspace/pset4/jpg $ check50 2015.fall.pset4.recover recover.c
:) recover.c exists
:) recover.c compiles
:( recovers 000.jpg correctly
   \ expected an exit code of 0, not standard error of "/opt/sandbox50/bin/run.sh: line 31:  36..."
:( recovers middle files correctly
   \ expected an exit code of 0, not standard error of "/opt/sandbox50/bin/run.sh: line 31:  36..."
:( recovers last file correctly
   \ expected an exit code of 0, not standard error of "/opt/sandbox50/bin/run.sh: line 31:  36..."
https://sandbox.cs50.net/checks/7ada1b604084487cb4e1a624d2878228
storm12345@ide50:~/workspace/pset4/jpg $

1 Answer

0 votes
answered Jun 18, 2016 by Faïza Harbi (11,960 points)
Okay, so , there are a few things I have issues with.

Your check50 is yelling at you for a good reason: You are trying to store a string in an int (n) on line 36. that is not ok, especially since you created an array of char size 30 (too big btw) to do that. Hence your segfault.

Now, just like resize, it's not because you can see the image that you actually recovered the jpg file.

It is said that here, our jpgs are sort of divided in chunks of 512 BYTES, it doesn't mean that a jpg is actually 1 BYTE. but you only copy 1 BYTE.

Also, you're gonna have an issue with the way you get out of your while loop.

Why going through an int called c (that you only change after and even that I m not sure is ok: you gotta deal with the actual type you are given or else your code is really messy and hard to debug) that eventually once you have recovered all your jpg will still be not EOF with the method you use ?

line 33 to 44: no matter what the state of outfile you, in both cases end up writing in it. That is something quite bizarre don't you think? Especially since you actually never read any file ever. you cannot write something you didn't read. If you don't read your infile, you cannot get the data from it at the right moment (the 4 sequences you use memcmp for).

Remember: You have no way of knowing when a jpg ends. You only know when one starts. Lucky us, we are told that they are stored one after another.

Go back and open copy.c and re-read the code as for how you read and write from one file to another.
commented Jun 18, 2016 by storm (290 points)
I changed few lines of code but I am not able to understand what are you trying to say about Int c and also when i removed int n and declared a string at its place, the compiler gives me the following error-
error: incompatible integer to pointer conversion assigning to 'string' (aka 'char *') from 'int' [-Werror,-Wint-conversion]
           s= sprintf(filename, "%03d.jpg", filenumber);
commented Jun 18, 2016 by Faïza Harbi (11,960 points)
My question is: why are you trying to assign sprintf to anything? sprinf will by itself write in filename "000.jpg\0" if filenumber is 0 . just call the function sprintf all by itself, no need to assign that function to anything.
commented Jun 18, 2016 by storm (290 points)
Ohh ok I have corrected it and also reduced the size of array to 8 and my new code is-
#include<stdio.h>
 #include<string.h>
 

int main(int argc, char* argv[])
{
    // TODO
    
    FILE*infile = fopen("card.raw", "r");
    char buf[512] = {0};
    char jpeg[4] = {0xff, 0xd8, 0xff, 0xe0};
    char jpeg1[4] = {0xff, 0xd8, 0xff, 0xe1};
    char filename[8];
    int filenumber = 0;
    int c = 0;
    FILE* outfile;
    
    while(c != EOF)
    {
       for (int i = 0; i < 512; i++)
       {
            buf[i] = c = fgetc(infile);
        }
       
       if(memcmp(buf, jpeg, 4) == 0 || memcmp(buf, jpeg1, 4) == 0)
       {
           if(outfile)
           fclose(outfile);
           
       
           
           sprintf(filename, "%03d.jpg", filenumber);
           filenumber++;
           
           outfile = fopen(filename, "w");
           if(!outfile)
           {
               printf("error opening outfile\n");
               return 1;
           }
           else if(outfile)
       {
            fwrite(buf, 1, 512, outfile);
       }
       }
       
    }
    fclose(infile);
    if(outfile)
    fclose(outfile);
   
    return 0;
    
}
and i am still getting the same error
commented Jun 18, 2016 by Faïza Harbi (11,960 points)
okay, so: At no point whatsoever are we told that what we are recovering is of type char right? we are told about BYTE, yet, your recovering array is of type char... you may want to change that. Same goes for your 2 arrays containing hexadecimal values: It's not logical to put "numbers" in arrays of chars.

Also, still something not ok with NOT reading your infile: how can you write something you do not read? What do you exactly recover then?

You open, you read what you've opened, if you find the proper sequence of 4 hexa values you write, but you can only find them by reading them. Opening the file doesn't make it all read, you have to do it.

Still not happy with your int c btw.
commented Jun 18, 2016 by storm (290 points)
removed char array and int c and the new code-
#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>
#include <string.h>


int main(int argc, char* argv[])
{
    // TODO
    char* infile = "card.raw";
    
    FILE* inptr = fopen(infile , "r");
    uint8_t buf[512] = {0};
    uint8_t jpeg[4] = {0xff, 0xd8, 0xff, 0xe0};
    uint8_t jpeg1[4] = {0xff, 0xd8, 0xff, 0xe1};
    char filename[8];
    int filenumber = 0;
    
    FILE* outfile;
    
    while(fread(buf, 512, 1, inptr) != 0)
    {
       for (int i = 0; i < 512; i++)
       {
            buf[i] = fgetc(inptr);
        }
       
       if(memcmp(buf, jpeg, 4) == 0 || memcmp(buf, jpeg1, 4) == 0)
       {
           if(outfile)
           fclose(outfile);
           
       
           
           sprintf(filename, "%03d.jpg", filenumber);
           filenumber++;
           
           outfile = fopen(filename, "w");
           if(!outfile)
           {
               printf("error opening outfile\n");
               return 1;
           }
           else if(outfile)
       {
            fwrite(buf, 1, 512, outfile);
       }
       }
       
    }
    fclose(inptr);
    if(outfile)
    fclose(outfile);
   
    return 0;
    
}
but its still getting the same error
commented Jun 18, 2016 by Faïza Harbi (11,960 points)
char* infile? really?
and again, why are you only reading one chunk, one BYTE?

You need to take a pen and a paper and write down what your program does, not what you think it does nor what you want it to do. You are close, so it's really a matter of logic.

also, if I were you, I wouldn't bother with memcmp and type the full OR thing, just in case: it's longer, but it's clearer. Start by commenting your code, then pen and paper to see if you actually do what you think you are doing
commented Jun 18, 2016 by storm (290 points)
thank you very much finally got it done...
Welcome to CS50xHelpers Q&A, where you can ask questions and receive answers from other members of the community.

197 questions

248 answers

217 comments

4,843 users

1 Online
0 Member And 1 Guest
...