CODEWITHSUNDEEP
ccs50
Qunoot K
Qunoot K

Reputation: 71

CS50 recover segmentation fault

This is Recover from pset4 CS50. There is no error when I compile it. When I run the code I get segmentation fault. I do not understand what exactly is the mistake. I have looked up solutions related to questions on segmentation fault posted by others but they don't seem to solve my problem.

Can somebody explain whats wrong and how do I solve it. 

#include <stdio.h>
#include <stdint.h>
typedef uint8_t BYTE;

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

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

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

BYTE buffer[512];
FILE *outFile = NULL;
int imageNum = 0;
char fileName[8];

while (!feof(inFile))
{
     fread(buffer, 1, sizeof(buffer), inFile);
     if (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[4] & 0xf0) == 0xef0)
     {
         if (imageNum > 0)
         {
            fclose(outFile);
         }
         imageNum++;
         sprintf(fileName, "%03i.jpg", imageNum);
         outFile = fopen(fileName, "w");
     }
    if (outFile != NULL)
    {
        fwrite(buffer, 1, sizeof(buffer), outFile);
    }
}
fclose(outFile);
fclose(inFile);
return 0;
}

Upvotes: 0

Views: 515

Answers (3)

Qunoot K
Qunoot K

Reputation: 71

I found the solution to problem above by changing these lines of code

while (!feof(inFile))

to 

while (fread(buffer, sizeof(buffer), 1, inFile))

and

fread(buffer, 1, sizeof(buffer), inFile);
fwrite(buffer, 1, sizeof(buffer), outFile);

to 

fread(buffer, sizeof(buffer), 1, inFile);
fwrite(buffer, sizeof(buffer), 1, outFile);

Also the main issue was in my if condition which I changed from

if (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[4] & 0xf0) == 0xef0)

to

 if (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[3] & 0xf0) == 0xe0)

and the last change

imageNum++;
sprintf(fileName, "%03i.jpg", imageNum);
outFile = fopen(fileName, "w");

to

sprintf(fileName, "%03i.jpg", imageNum);
outFile = fopen(fileName, "w");
imageNum++;

Thank you all for your suggestions and help.

Upvotes: 1

ryyker
ryyker

Reputation: 23226

The reason for the segfault is likely this declaration:

char fileName[8];

For filenames greater than 999, fileName will overflow.

i.e. for imagenum >= 1000

sprintf(fileName, "%03i.jpg", imageNum);//produces 8 characters + NULL == 9

...will produce "1000.jpg" which is a buffer overflow, thus segfault.

Making fileName larger is the fix: 

char filename[20];//or larger as needed.

Also,
per definitions of fwrite() and fread(), the following function arguments are misplaced:

fread(buffer, 1, sizeof(buffer), inFile);
fwrite(buffer, 1, sizeof(buffer), outFile);
               ^        ^

Should be:

fread(buffer, sizeof(buffer), 1, inFile);
fwrite(buffer, sizeof(buffer), 1, outFile);
                     ^         ^

Upvotes: 1

Eric Postpischil
Eric Postpischil

Reputation: 223693

fileName is defined to have eight elements in char fileName[8];, but sprintf(fileName, "%03i.jpg", imageNum); writes more than eight characters once imageNum exceeds 999.

Upvotes: 0

Related Questions