CODEWITHSUNDEEP
cfile-io
mu_sa
mu_sa

Reputation: 2725

File reading in C

I am new to C and the problem i am facing is that that i have a binary file which is unpacked (i.e its in a certain format)..

What i am suppose to do is to pack it and unpack it again and see if its equal to the original unpacked version.

One thing worth mentioning: i have been told the packing (i.e convert to packed) and unpacking(i.e convert to unpacked) function works well.. Just want to confirm it for myself and learn a bit of C...

I have two points where i think i am doing mistake

1 : the way i am reading the file

2 : I am not properly considering the variable type of packed and unpacked (i.e for packed it is unsigned char * and for unpacked it is short * ) 

int main(void) {

FILE *fp;

unsigned char* packed ;
short* unpacked;
size_t result;
int fileSize;

fp = fopen(FILENAME, "rb");

fseek (fp , 0 , SEEK_END);
fileSize = ftell (fp);
rewind (fp);

unpacked = (short*) malloc (sizeof(char)*fileSize);

result = fread(unpacked,1,fileSize,fp);

short *originalUnpacked = unpacked;


convert_to_packed(&unpacked, &packed);

convert_to_unpacked(&unpacked, &packed);

if (originalUnpacked == unpacked)
{
    puts ("Thats it !!");

}

fclose(fp );
return EXIT_SUCCESS;
}

Upvotes: 0

Views: 254

Answers (2)

Skizz
Skizz

Reputation: 71060

You need to get a better understanding of what pointers are. Think of a pointer as an address you'd put on a letter, and the data, or memory, as the house. If I make a copy of the address there are now two letters but only one house, both letters refer to the same house and I can use either to get to the house. Now, to duplicate the house requires a lot more work than just copying the address on the letter. You need a new house and a new letter with a new address.

unpacked = (short*) malloc (sizeof(char)*fileSize);
  ^^                  ^^
the letter          building the house


short *originalUnpacked = unpacked;
         ^^                  ^^
       new letter         old letter = copying the letter (address) not the house!


if (originalUnpacked == unpacked)
        ^^                 ^^
    new letter          old letter - neither changed so are equal (same address)

You gave this address to the conversion functions convert_to_packed and convert_to_unpacked which then overwrote the original data because you forgot to build the new house to keep the original data in.

To fix the code, you need to build a new house and use it to store the result of the packing and unpacking. You then need to compare the contents of the two houses and not the letters that address them.

 short newUnpacked = (short*) malloc (sizeof(char)*fileSize);

 convert_to_packed(&unpacked, &packed);
 convert_to_unpacked(&unpacked, &newUnpacked); // put unpacked data into new house!

 if (memcmp (unpacked, newUnpacked, filesize) == 0) // comparing contents of houses!

Upvotes: 0

unwind
unwind

Reputation: 399703

This code is very broken.

You're comparing pointers (originalUnpacked == unpacked) when you should be comparing memory. And why are you mixing short * and unsigned char * pointers? If the data is a binary "blob", you should probably only use the latter.

To compare memory, use the standard memcmp() function.

Upvotes: 3

Related Questions