0

I'm very confused why this code is not giving any output: Code:

#include <stdio.h> #include <stdlib.h> #include <stdint.h> typedef uint8_t BYTE; long int findSize(char file_name[]) { // opening the file in read mode FILE* fp = fopen(file_name, "r"); // checking if the file exist or not if (fp == NULL) { printf("File Not Found!\n"); return -1; } fseek(fp, 0L, SEEK_END); // calculating the size of the file long int res = ftell(fp); // closing the file fclose(fp); return res; } void copy(BYTE newarr[], BYTE oldarr[]) { for (int i = 0; i < 512; i++) { newarr[i] = oldarr[i]; } } int main(int argc, char *argv[]) { if (argc < 2 || argc > 2) { printf("Usage: ./recover image"); return 1; } FILE *card = fopen(argv[1], "r"); if (card == NULL) { printf("Unable to open: %s\n", argv[1]); return 2; } else { BYTE buffer[512]; int file_size = findSize(argv[1])/512; BYTE block[file_size][512]; int block_counter = 0; while (fread(&buffer, sizeof(BYTE), 512, card)) { copy(block[block_counter], buffer); } int file_count = 0; char filename[17]; FILE *out = NULL; for (int block_number = 0; block_number < file_size; block_number++) { if (block[block_number][0] == 0xff && block[block_number][1] == 0xd8 && block[block_number][2] == 0xff && (block[block_number][3] & 0xf0) == 0xe0) { if (file_count > 0) { fclose(out); } sprintf(filename, "%03i.jpg",file_count); out = fopen(filename, "w"); fwrite(&block[block_number], sizeof(BYTE), 512, out); file_count++; } else if (file_count > 1) { fwrite(&block[block_number], sizeof(BYTE), 512, out); } } if (out != NULL) { fclose(out); } } if (card != NULL) { fclose(card); } return 0; }
Improve this question

1 Answer 1

Reset to default
0

For starters, the first while statement isn't incrementing block_counter, so the code is just overwriting the first 512 bytes of block over and over until it only contains the last 512 bytes of the input file. Since there's no signature found ever, no output is generated.

There may be other issues, but this will get you back on track and give you the chance to work on them, if they exist. ;-)

I didn't do a deep dive in your program, but there are a few things that jump out at me. Why did you decide to read in the entire file before processing it? This requires an undetermined amount of memory (only determined at run time), and potentially more memory than is available. Why not just read in a 512 byte block, process it and then read the next block? This would require only 512 bytes of memory plus other variables.

Programming note: Best practices.

The code validates the input vars and will either terminate the program or continue. In this scenario, it is both not necessary and bad practice to use an else clause. The else clause surrounds the remainder of the program. It would be easy to introduce a bug in the code because the surrounding else clause is overlooked. Code should be compartmentalized as much as possible. Interdependencies should be reduced as much as possible too. Wrapping the rest of the code with an else clause introduces a massive interdependence that isn't needed.

If this answers your question, please click on the check mark to accept. Let's keep up on forum maintenance. ;-)

Improve this answer
1
  • It somewhat worked. All the pictures were correct except for the first one. It was just fully blank. And thanks for your tip. I have actually completed week 4 with another code of mine. I was just asking that why is this code not working Commented Nov 8, 2020 at 5:32

You must log in to answer this question.