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
– user3629249, Commented Sep 2, 2015 at 6: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
– user3629249, Commented Sep 2, 2015 at 6:36
the call to fread() immediately followed by while( fread() ) means the first JPG in the file will be missed — user3629249
– user3629249, Commented Sep 2, 2015 at 6:38
when calling the system function: fopen(), always check (!=NULL) the returned value to assure the operation was successful — user3629249
– user3629249, Commented Sep 2, 2015 at 6:41
what makes you think the jpg file will be an exact multiple of 512 bytes? Suggest following the details in this page: <stackoverflow.com/questions/2517854/…> as it has links to an appropriate library, discussion of the jpeg file layout, etc. — user3629249
– user3629249, Commented Sep 2, 2015 at 6:58

timrau · Accepted Answer · 2015-09-01 17:00:48Z

1

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.

answered Sep 1, 2015 at 17:00

timrau

23.1k4 gold badges55 silver badges67 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Caleb Jay · Accepted Answer · 2015-09-03 00:30:14Z

0

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)
{
    ...
}

answered Sep 3, 2015 at 0:30

Caleb Jay

2,2094 gold badges43 silver badges77 bronze badges

Collectives™ on Stack Overflow

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

2 Answers 2

Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related