CS50: Segmentation fault pset4 recover

Question

I've written this below code but the error is something to do with the char array of size 8. I am not sure why it's happening. I also tried changing the size of the array. I still get the same error. More about the file naming method: The files you generate should each be named ###.jpg, where ### is a three-digit decimal number, starting with 000 for the first image and counting up. The file name should have enough memory to store these digits.

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

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

 const int  BLOCK_SIZE = 512;
 typedef uint8_t BYTE;

  if (argc !=  2)
    {
        printf("Usage: ./recover image\n");
        return 1;
    }


  FILE *infile = fopen(argv[1], "r");
  if(infile == NULL)
  {
    printf("Could not open \n");
    return 2;
  }

  int index = 0;
  int flag = 0;
  FILE *outfile;
  char file[8];
  BYTE buffer[BLOCK_SIZE];


 while(fread(buffer, sizeof(BLOCK_SIZE), 1, infile) == 0)
 { //beginning of jpg
  if (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[3] & 0xf0) == 0xe0)
  {
    // if Ist jpeg
    if (flag == 0)
    {
      sprintf(file, "%03i.jpg", index);
      outfile = fopen(file, "w");
      fwrite(buffer, sizeof(BLOCK_SIZE), 1, outfile);
      flag = 1;
      index++;

    }

    else if(flag == 1)
    {
      fclose(outfile);
      sprintf(file, "%03i.jpg", index);// pad it with 0s and is 3 chars wide and the last char gets index
      outfile = fopen(file, "w");
      fwrite(buffer, sizeof(BLOCK_SIZE),1, outfile);
      index++;

    }

  }

  else
  {
     fwrite(buffer, sizeof(BLOCK_SIZE), 1, outfile);

  }


}
fclose(outfile);
fclose(infile);
}

I also tried valgrind from cs50's help.

==2500== Memcheck, a memory error detector
==2500== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==2500== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info
==2500== Command: ./recover card.raw
==2500== 
==2500== Use of uninitialised value of size 8
==2500==    at 0x4A69F5B: fclose@@GLIBC_2.2.5 (iofclose.c:48)
==2500==    by 0x4013C4: main (recover.c:67)
==2500==  Uninitialised value was created by a stack allocation
==2500==    at 0x401190: main (recover.c:6)
==2500== 
==2500== Use of uninitialised value of size 8
==2500==    at 0x4A6A05D: fclose@@GLIBC_2.2.5 (iofclose.c:51)
==2500==    by 0x4013C4: main (recover.c:67)
==2500==  Uninitialised value was created by a stack allocation
==2500==    at 0x401190: main (recover.c:6)
==2500== 
==2500== Invalid read of size 8
==2500==    at 0x4A6A06D: fclose@@GLIBC_2.2.5 (iofclose.c:51)
==2500==    by 0x4013C4: main (recover.c:67)
==2500==  Address 0x8 is not stack'd, malloc'd or (recently) free'd
==2500== 
==2500== 
==2500== Process terminating with default action of signal 11 (SIGSEGV)
==2500==  Access not within mapped region at address 0x8
==2500==    at 0x4A6A06D: fclose@@GLIBC_2.2.5 (iofclose.c:51)
==2500==    by 0x4013C4: main (recover.c:67)
==2500==  If you believe this happened as a result of a stack
==2500==  overflow in your program's main thread (unlikely but
==2500==  possible), you can try to increase the size of the
==2500==  main thread stack using the --main-stacksize= flag.
==2500==  The main thread stack size used in this run was 10485760.
==2500== 
==2500== HEAP SUMMARY:
==2500==     in use at exit: 480 bytes in 2 blocks
==2500==   total heap usage: 3 allocs, 1 frees, 4,576 bytes allocated
==2500== 
==2500== 8 bytes in 1 blocks are still reachable in loss record 1 of 2
==2500==    at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==2500==    by 0x40122E: main (recover.c:28)
==2500== 
==2500== 472 bytes in 1 blocks are still reachable in loss record 2 of 2
==2500==    at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==2500==    by 0x4A6AAAD: __fopen_internal (iofopen.c:65)
==2500==    by 0x4A6AAAD: fopen@@GLIBC_2.2.5 (iofopen.c:86)
==2500==    by 0x4011EA: main (recover.c:18)
==2500== 
==2500== LEAK SUMMARY:
==2500==    definitely lost: 0 bytes in 0 blocks
==2500==    indirectly lost: 0 bytes in 0 blocks
==2500==      possibly lost: 0 bytes in 0 blocks
==2500==    still reachable: 480 bytes in 2 blocks
==2500==         suppressed: 0 bytes in 0 blocks
==2500== 
==2500== For lists of detected and suppressed errors, rerun with: -s
==2500== ERROR SUMMARY: 3 errors from 3 contexts (suppressed: 0 from 0)
/etc/profile.d/cli.sh: line 95:  2500 Segmentation fault      valgrind ./recover card.raw

score 1 · Answer 1 · answered Sep 25 '21 at 13:29

char[8] should be sufficient for the file name. But that is not where the problem lies.

Have a look at what fread returns from man fread

On success, fread() and fwrite() return the number of items read or written. This number equals the number of bytes transferred only when size is 1. If an error occurs, or the end of the file is reached, the return value is a short item count (or zero).

Or in other words, fread returns 0 when there is an error, or end of file is reached.

What will fread() return on the first execution in the while block? Hint: it won't be zero. Where will program control go? You can use debug50 with a breakpoint on the while statment to get a "visual" on flow of control.

After that is corrected, there is another seg fault in this program's future. You will want to debug again, with a breakpoint at the if test for jpg signature. If the first block read is not a signature (as with the supplied card.raw), where does control go next?

Thanks for reaching out! I figured out the segmentation error. Also `while (fread (buffer, sizeof(BLOCK_SIZE), 1, infile) == 0)` should be `while (fread(buffer, sizeof(BLOCK_SIZE), 1, infile`) . `What will fread() return on the first execution in the while block?` Its not going to get into the while block since there are no errors or EOF is not reached — Annie, Sep 25 '21 at 14:27

score 0 · Answer 2 · answered Sep 25 '21 at 13:58

I figured out why. I didn't realize the use case: what if the very first block of 512 bytes in the memory card does not have a jpeg header? According to the code I've written, even if the jpeg header in the first block is not present, it's gonna write to a file that does not exit which is causing a segmentation fault. (the last else block).

By changing the last else block:

else 
      {
         fwrite(buffer, sizeof(BLOCK_SIZE), 1, outfile);
    
      }

to

 else if (flag == 1)
      {
         fwrite(buffer, sizeof(BLOCK_SIZE), 1, outfile);
    
      }

access is forbidden to write to a file that does not exist.

CS50: Segmentation fault pset4 recover

2 Answers2