0

The code compiles perfectly. However, it triggers a segmentation fault which I imagine is due to a wrong use of pointers. This is the code:

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

int main(int argc, char *argv[])
{
    //counts how many JPEGS have been found
    int jpeg_counter = 0;
    //stores the data to write on the img
    char new_filename[512];
    //set a buffer for reading
    int buffer[512];
    //file to write the images
    FILE * img;
    //ensure proper usage
    if (argc != 2)
    {
        printf("Usage: ./recover filename\n");
        return 1;
    }

    else
    {
        //card.raw filename
        char *filename = argv[1];
        //opening card.raw
        FILE *f = fopen(filename, "r");
        //iterate through card.raw every 512 bytes until end of card
        while (!feof(f))
        {
            //read through 512 bytes of card, storing it in the buffer
            fread(&buffer, sizeof(char), 512, f);
            //checking the start pf a JPEG file
            if (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[3] & 0xf0) == 0xe0)
            {
                jpeg_counter++;
                //if the first block of a JPEG, write the first 512 bytes
                if (jpeg_counter == 1)
                {
                    sprintf(new_filename, "%03i.jpg", jpeg_counter);
                    img = fopen(new_filename, "w");
                    fwrite(&new_filename, sizeof(char), 512, img);
                }
                //if second and following jpegs, write the first 512 bytes
                else
                {
                    fclose(img);
                    sprintf(new_filename, "%03i.jpg", jpeg_counter);
                    img = fopen(new_filename, "w");
                    fwrite(&new_filename, sizeof(char), 512, img);
                }
            }
            
            else
            {
                //if already found a JPEG
                if (jpeg_counter > 1)
                {
                    //keep writing the rest of the 
                    fwrite(new_filename, sizeof(char), 512, img);
                }
            }
        }
    fclose(f);
    fclose(img);
    }
}

I am unable to spot where can I improve my code to work. Hopefully, I can get some hints/tips on what should I change or next steps.

Thank you :)

1

A few things to think about (not an exhaustive list):

  1. You want a 512 byte buffer, but you declare an array of 512 4-byte integers.
  2. new_filename is an actual 512 byte variable. A problem arises when you write your filename to it, then try to write it to the file in turn. This means you're trying to write the characters of the filename followed by 504 bytes of unknown data to the file. Perhaps you meant to write the buffer that you read earlier to the file instead? As such, new_filename probably doesn't need to be 512 bytes long.
  3. Your segmentation fault is probably arising at your fread. When you pass an array by name with no index (in this case, buffer), it will evaluate as a pointer to the first element of the array, which is what you want. However, you are using the reference operator &. Rather than passing a pointer to an allocated 512 bytes of memory, you're now passing the address of said pointer. The function will attempt to start writing here and consequently into memory you're not supposed to be writing to.
  4. The same issue with the reference operator applies to the fwrite calls as well. Rather than writing the actual data in your array to the file, you'll get a random 512 bytes starting wherever in memory the address of the array is being held.
  5. Not critical, but for code clarity the first else statement is unnecessary. If the number of arguments is incorrect, the return statement will exit the function immediately anyway.
7
  • Hi Sentox, thank you for your response! I have applied the errors you mentioned, however I am sitll getting seg fault. What should I do next? I have edited the code so you can see what is the progress so far. – Jaime Nov 9 '20 at 1:36
  • @Jaime You're very close now! Where you declare buffer, try changing "int" to "unsigned char" and see how you go. – Sentox Nov 9 '20 at 1:45
  • Hey @sentox, that was exactly what I needed! I was completely unaware that unsigned char existed. I would have been tweaking around the code for days without realzing the problem. – Jaime Nov 9 '20 at 2:01
  • @Jaime To elaborate on why you had a seg fault, while your buffer was 512 bytes as a result of the malloc call, in declaring it as type int, it causes the [n] operator to treat it in 4 byte increments. Thus when you check buffer[0], buffer[1], etc to see if it's a jpeg header, you weren't testing the first four bytes individual bytes, you were checking the first four "ints" (aka groups of 4 bytes each). As a result your program never found a jpeg. The segfault arose with the call to fclose(img), since img never got opened in the first place. Hope this helps. – Sentox Nov 9 '20 at 2:02
  • That makes sense. Why does it need to be an unsigned char, instead of a char? – Jaime Nov 9 '20 at 2:20

You must log in to answer this question.

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