Recover.c successfully recovers several photos before a segmentation fault

Question

I read several questions related to the segmentation fault but still cannot figure out what exactly is wrong with my code. Basically, what happens is that the program recovers around 10 photos correctly and then it crashes at line 36 (just after the initialization of the counter) with a "No such file or directory" message at gdb. Although it recovers clearly these photos it doesn't pass the check50 (gets "killed by server"). Thank in advance for any input! :)

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

unsigned char buffer[512];

//open the file (card.raw)

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

//create and open output file where we're going to write
char filename[8]; 

FILE* outptr= NULL;
outptr=fopen(filename,"w");
int counter = 0;

while (!feof(file))
{

   fread(&buffer,512,1,file);

  //track for jpg signature
   if(buffer[0]==0xff && buffer[1]==0xd8 && buffer[2]==0xff && (buffer[3]==0xe0 || buffer[3]==0xe1))
   {

      //close previous file if opened
      if(outptr!=NULL)
      {
         fclose(outptr);
      }
      //open a new one to write to
      sprintf(filename,"00%d.jpg", counter);
      outptr=fopen(filename, "w");


       fwrite(&buffer,512,1,outptr);
       counter++;
   }
   //if no jpg signature is found yet, 
   else 
   {

       fwrite(&buffer,512,1,outptr);
    }


}
fclose(file);
  // close outfile
fclose(outptr);


return 0;
}

Answer 1

I see a few problems. First, you open an output file before you have populated the variable that is supposed to hold the filename. It uses the random garbage data in the variable for the file name, creating a file with a very strange name.

Second, look at these two lines:

char filename[8]; 
...
sprintf(filename,"00%d.jpg", counter);

Here's your current seg fault. When you get to 10, your filename is 0010.jpg. When you count the '\0' string terminator, the sprintf will populate 9 chars, one longer than the array. The next line will attempt to access that whole string, go beyond the end of filename and trigger the seg fault. If you review the class content, you'll see that there is a better way to format and create the file name.

Finally, another potential seg fault. If you don't create the initial output file before the while loop, you'll need to make sure that the else clause at the bottom doesn't write the leading garbage data to anything, an easy task to implement.

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