2

When I compile and run my program, I get a segmentation fault. Valgrind doesn't show any problems. Any thoughts? I'm going crazy trying to figure out the problem.

The point of the program is to recover JPEGS from a "forensic image" called card.raw . When I run check50 on the code the program fails to meet the requirements due to a seg fault.

This is my code:

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


int main(int argc, char *argv[])
{
int jpeg_count = 0;

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

FILE *original;
original = fopen(argv[1], "r");

if(original == NULL)
{
    printf("Error! Cannot open file.\n");
    return(1);
}

unsigned char buffer[512];
char *filename = malloc(8);
fread(buffer, sizeof(buffer), 512, original);

FILE *img; 

while(1)
{
    if(buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[3] &0xf0) == 0xe0)
    {
        if(jpeg_count == 0)
        {
            sprintf(filename, "%03i.jpg", 0);
            img = fopen(filename, "w");
            fwrite(buffer, sizeof(buffer), 512, img);
            jpeg_count++;
        }
        else if(jpeg_count > 0)
        {
            fclose(img);
            sprintf(filename, "%03i.jpg", jpeg_count);
            img = fopen(filename, "w");
            fwrite(buffer, sizeof(buffer), 512, img);
            jpeg_count++;
        }
    
    }
    if(feof(original))
    {
        fclose(img);
        fclose(original);
        free(filename);
        break;
            
    }             
}

}
Community
  • 1
  • 1
  • You should run your program in a debugger. It should tell you where the segfault happens. Then check all your variables for proper values. Where do you read anything after the first block from the file? – Gerhardh Jun 29 '20 at 16:58
  • Some hints: There is no need to dynamically allocate memory for `filename`. If counter is greater than `999` it will not provide enough space. You also should check return value when opening any new file. And BTW: What do you expect to happen if the read block does not contain the header with the magic bytes? – Gerhardh Jun 29 '20 at 17:02
  • 1
    Your `fread` is incorrect. Referring to `man fread`, the total number of bytes read is `size * nmemb`. In your `fread`, `size` is `sizeof(buffer)` which is 512 and `nmemb` is 512, so you are saying read `512 * 512` bytes, which is 262,144 bytes--much larger than the size of `buffer`. This causes undefined behavior (e.g. segfault). In your case, `buffer` is an array of `char`, which has a size of 1, and you have 512 elements. The idiomatic form should be: `fread(buffer,1,sizeof(buffer),original)` – Craig Estey Jun 29 '20 at 17:32
  • You should also check the return value of `fread`. How would you know if it failed without? – Gerhardh Jun 30 '20 at 06:05
  • Perhaps [cs50 pset4 Recover - why does writing entire file to memory fail check50?](https://stackoverflow.com/a/62608167/3422102) answers your question. – David C. Rankin Jul 30 '20 at 00:38

0 Answers0