sprintf() usage, and gdb reporting program breaking out of loop early?

Question

I'm building a program for a class that pulls all bytes from a raw file in blocks of 512 bytes, searches the beginning bytes of each block for JPG signatures, and upon finding JPG signatures, writes out a JPG image. I have successfully pulled one JPG from the raw file, however the course indicates there are several more. After debugging in GDB, I've noticed that the variable "jpgname" never seems to have a value, and also, stepping through the program, I regularly step to the "fclose(inptr)" line, as if it were within the main while() loop. Am I using GDB incorrectly? Am I using sprintf() incorrectly? I would appreciate any feedback on this issue.

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

typedef uint8_t  BYTE;


int main(void)
{
     BYTE jpg1[4] = {0xff, 0xd8, 0xff, 0xe0};
     BYTE jpg2[4] = {0xff, 0xd8, 0xff, 0xe1};
     int open = 0;
     FILE* outp;
     char* jpgname = malloc(sizeof(char) * 8);
     int counter;

     //TODO - MAKE sure this isn't a bunch of garbage data

    //open card file
    FILE* inptr = fopen("card.raw", "r");

    if (inptr == NULL)
    {
        printf("Could not open file\n");
        return 2;
    }

    BYTE buffer[512];
    fread(buffer, 512, 1, inptr);

    //repeat until end of card    
    while(fread(buffer, 512, 1, inptr) > 0)
    {                
            counter = 0;


            //start of new jpg?
            if((buffer[0] == jpg1[0] && buffer[1] == jpg1[1] && buffer[2] == jpg1[2] && buffer[3] == jpg1[3]) || (buffer[0] == jpg2[0] && buffer[1] == jpg2[1] && buffer[2] == jpg2[2] && buffer[3] == jpg2[3]))
            {

                sprintf(jpgname, "%d.jpg", counter); 
                if (open == 1)
                {
                    //close
                    fclose(outp);
                    //name new file
                    outp = fopen(jpgname, "w");
                    //write new
                    fwrite(buffer, sizeof(buffer), 1, outp);
                    counter++;
                }
                else if (open == 0)
                {
                    //write new file
                    outp = fopen(jpgname, "w");
                    fwrite(buffer, sizeof(buffer), 1, outp);
                    open = 1;
                    counter++;
                }
            }
            else
            {
                if(open == 1)
                {
                    fwrite(buffer, sizeof(buffer), 1, outp);
                }
            }
    } 

    fclose(outp);
    fclose(inptr);        
   //close any remaining files
}

the expression: `sizeof(char)` is defined as 1 and has absolutely no effect on the value sent to malloc(). Suggest declutter the code by removing that expression. Always check (!=NULL) the returned value from malloc() to assure the operation was successful — user3629249, Sep 02 '15 at 06:33
this line: `printf("Could not open file\n");` is not a good way to handle an error return from a system function. instead use `perror( "some text" );` as perror will output the 'some text' and the system error message as would be selected by the 'errno' variable. — user3629249, Sep 02 '15 at 06:36
the call to `fread()` immediately followed by `while( fread() )` means the first JPG in the file will be missed — user3629249, Sep 02 '15 at 06:38
when calling the system function: `fopen()`, always check (!=NULL) the returned value to assure the operation was successful — user3629249, Sep 02 '15 at 06:41
what makes you think the jpg file will be an exact multiple of 512 bytes? Suggest following the details in this page: as it has links to an appropriate library, discussion of the jpeg file layout, etc. — user3629249, Sep 02 '15 at 06:58
this web page: describes the jpeg format. I think the two byte strings you will most be interested in are 0xFFD8 (start of image) and 0xFFD9 (end of image) — user3629249, Sep 02 '15 at 07:16
Appreciate the help. I will work on these during lunch today and see if that sorts it. The RAW file is class-provided and so the professor broke up each file into 512 byte "blocks" as he describes it. Aren't there two possible start of image indicators? — Caleb Jay, Sep 02 '15 at 13:01

score 1 · Accepted Answer · answered Sep 01 '15 at 17:00

BYTE buffer[512];
fread(buffer, 512, 1, inptr);

//repeat until end of card    
while(fread(buffer, 512, 1, inptr) > 0)

You should check why you decided to drop the beginning 512 bytes read in.

FILE* inptr = fopen("card.raw", "r");

You should open input file and output files in binary mode like:

FILE* inptr = fopen("card.raw", "rb");

outp = fopen(jpgname, "wb");

Otherwise, the data read in or written out may be messed.

score 0 · Answer 2 · answered Sep 03 '15 at 00:30

While I appreciate everyone's input and have made my code better as such, the error causing the program to not work at all was due to my counter being reset to 0 at the beginning of the loop.

while(fread(buffer, 512, 1, inptr) > 0)
{                
        counter = 0;
        ...
}

should be

counter = 0;
while(fread(buffer, 512, 1, inptr) > 0)
{
    ...
}

sprintf() usage, and gdb reporting program breaking out of loop early?

2 Answers2