0

Please have a look at my recover.c and please help me

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

int main(int argc, char *argv[])
{

char *infile=argv[1];

if(argc!=2)
{
    fprintf(stderr , "Usage : ./recover file.raw\n");
    return 1;

}

FILE *input = fopen(infile, "r");
if(input == NULL)
{
    fprintf(stderr, "Could not open %s\n" , infile);
    return 2;

}



 char *buffer[512];

 char *file="000";int i=0;

 while(fread(&buffer,1,512,input)==1) //to iterate over the ehole memory card
 {
   fseek(input, -512, SEEK_CUR);  //seeking the pointer to the beginning of the file because fread() was used in while() loop
   if(strcmp(buffer[0],"0xff")==0 && strcmp(buffer[1],"0xd8")==0  && strcmp(buffer[2],"0xff")==0 )// condition to check header of a JPEG
       {

           sprintf(file, "%03i.jpg",i++); //naming the JPEG files
           FILE *img =fopen(file,"w"); // creating the file


            while(fread(&buffer,1,512,input)==1) //reading the 512 B blocks until reached the EOF
            {

                fseek(input, -512, SEEK_CUR);//seeking the pointer because fread() was used in while() loop
                if(fread(&buffer,1,512,input)==1)
                {
                    fwrite(&buffer,1,512,img); //writing in output file as the buffer will be having the data of JPEG
                }

                else
                {
                    fseek(input, 1, SEEK_CUR); // seeking over EOF character
                }
            }
           fclose(img); //closing the image file after writing it

        }
 }

}

1

For starters, the code should check that argc == 2 before trying to do anything with argv[].

The while loop never runs. The return value from the fread is the number of elements read. Remember, the third parameter of the fread call is the number of elements that are supposed to be read, so it will return 512, not 1.

The allocation for *file will allocate 4 bytes total (3 chars plus the end of string marker) and then later, the sprintf call will write 8 bytes, overwriting whatever is stored next in memory. It would be better to just allocate an 8 char array here.

The if statement is using strcmp calls to compare the contents of the first 4 bytes of the buffer to specific values. This should be a direct == comparison, not a string comparison. As written, it is attempting to compare a one byte char to a 4 byte string.

There is no check for the 4th byte of the signature.

I really can't figure out the logic of repeated freads and fseeks to go backwards. This pset can be implemented simply by reading each block once and processing it, without moving backwards in the input file.

There may or may not be more issues, but there's a lot to fix here. I suggest a rethink of how to read through the input file without going backwards to reread anything.

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

1
  • Please can you suggest me any source to study more about fread() and fseek() functions ? – amethyst stan Jan 27 '18 at 9:35

You must log in to answer this question.

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