fwrite() writes the amount that i'm expecting but not the data that i want

Question

My task is to copy JPEGs from a file the intend here is to find the JPEGs and create a new file to put it, all JPEGs are store back to back and the "identifier" is always in the beggining of a 512 BYTE block. my problem shows only in the first file where it writes 128 bytes before the expected data from the buffer rendering the new file useless.

what i have tried:

Check the original file to see if the data came from it but it does not.
Malloc all the pointers to have "clear memory".
Reorganize the code.

any ideas?

{

typedef uint8_t BYTE;
int ok=0;

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

FILE *data= malloc(sizeof(BYTE)*512);
data=fopen(argv[1], "r");

if (data==NULL)
{
    printf("cannot open file.\n");
    return 1;
}

BYTE *buff=malloc(sizeof(BYTE)*512);
char filename[7]="000.jpg";
int filecount=0;
FILE *write=malloc(sizeof(BYTE)*512);


while (fread(buff, sizeof(BYTE), 512, data)==512)
{
    if (buff[0]==0xff && buff[1]==0xd8 && buff[2]==0xff && (buff[3] & 0xf0)==0xe0)
    {
        if (ok==1)
            fclose(write);

        if (filecount<10)
            sprintf(filename, "00%i.jpg",filecount);
        else
            sprintf(filename,"0%i.jpg",filecount);

        filecount++;
        ok=1;
        write=fopen(filename,"w");

    }

    if(ok==1)
        fwrite(buff, sizeof(BYTE), 512, write);
}
fclose(write);

`char filename[7]="000.jpg";` The array is too small to hold the data. — William Pursell, Jun 14 '21 at 04:24
"Cannot open file" is the canonical example of a useless error message. Try: `if( (data=fopen(argv[1], "r")) == NULL ){ perror(argv[1]); ...}` — William Pursell, Jun 14 '21 at 04:27
`FILE *write=malloc(sizeof(BYTE)*512);` ?!?!?!***! What do you expect this to do? — William Pursell, Jun 14 '21 at 04:40

William Pursell · Accepted Answer · 2021-06-14T04:42:00.520

Since filename is only large enough to hold a string of length 6, you have undefined behavior throughout your code. Use:

char filename[64] = "000.jpg";

To hold a string of length 7, you must have a buffer of size at least 8. (One extra to hold the terminating nul.) You could also write char filename[] = "000.jpg";, but there's no point in being stingy with arrays this small.

Also,

FILE *write=malloc(sizeof(BYTE)*512);

is clearly an error. I expect that is just a cut-n-paste error, but that should be:

FILE *write;
if( (write = fopen(filename, "w")) == NULL ){
        perror(filename);
        exit(EXIT_FAILURE);
}

Also, you can simplify construction of the incrementing names with:

snprintf(filename, sizeof filename, "%03i.jpg", filecount++);

fwrite() writes the amount that i'm expecting but not the data that i want

1 Answers1