Hey guys, I'm having some really odd problems, reading and writing using pack_fread, and pack_fwrite, the data just isn't coming back the same!
Parts of the code with the problems
| 1 | vars: |
| 2 | ACTOR * player; |
| 3 | ACTOR * npc[100]; |
| 4 | OBJECT * obj[100]; |
| 5 | |
| 6 | int map[256][256]; |
| 7 | int map_archive[20][256][256]; |
| 8 | OBJECT * map_obj_arc[20][100]; |
| 9 | ACTOR * map_npc_arc[20][100]; |
| 10 | int map_visited[20]; |
| 11 | |
| 12 | int dungeon_level; |
| 13 | int deepest_dungeon_level; |
| 14 | |
| 15 | |
| 16 | in save: |
| 17 | PACKFILE * fp = pack_fopen(filename, F_WRITE_PACKED); |
| 18 | if (fp) |
| 19 | { |
| 20 | int version = 1000; |
| 21 | |
| 22 | pack_fwrite(&version, 4, fp); |
| 23 | pack_fwrite(player, sizeof(ACTOR), fp); |
| 24 | pack_fwrite(&dungeon_level, 4, fp); |
| 25 | pack_fwrite(&deepest_dungeon_level, 4, fp); |
| 26 | |
| 27 | pack_fwrite(map, 256*256*4, fp); |
| 28 | pack_fwrite(npc, 100*sizeof(ACTOR), fp); |
| 29 | pack_fwrite(obj, 100*sizeof(OBJECT), fp); |
| 30 | |
| 31 | pack_fwrite(map_visited, 20*4, fp); |
| 32 | pack_fwrite(map_archive, 256*256*4*20, fp); |
| 33 | pack_fwrite(map_obj_arc, 20*100*sizeof(OBJECT), fp); |
| 34 | pack_fwrite(map_npc_arc, 20*100*sizeof(ACTOR), fp); |
| 35 | |
| 36 | pack_fclose(fp); |
| 37 | } |
| 38 | |
| 39 | in load: |
| 40 | PACKFILE * fp = pack_fopen(filename, F_READ_PACKED); |
| 41 | if (fp) |
| 42 | { |
| 43 | int version; |
| 44 | |
| 45 | pack_fread(&version, 4, fp); |
| 46 | if (version == 1000) |
| 47 | { |
| 48 | pack_fread(player, sizeof(ACTOR), fp); |
| 49 | pack_fread(&dungeon_level, 4, fp); |
| 50 | pack_fread(&deepest_dungeon_level, 4, fp); |
| 51 | |
| 52 | pack_fread(map, 256*256*4, fp); |
| 53 | pack_fread(npc, 100*sizeof(ACTOR), fp); |
| 54 | pack_fread(obj, 100*sizeof(OBJECT), fp); |
| 55 | |
| 56 | pack_fread(map_visited, 20*4, fp); |
| 57 | pack_fread(map_archive, 256*256*4*20, fp); |
| 58 | pack_fread(map_obj_arc, 20*100*sizeof(OBJECT), fp); |
| 59 | pack_fread(map_npc_arc, 20*100*sizeof(ACTOR), fp); |
| 60 | |
| 61 | } |
| 62 | |
| 63 | pack_fclose(fp); |
| 64 | } |
what's wrong here????
edit: I think i've got it.
some of the thing's like OBJECT * obj[100]; Contain null elements, so they probably break it when it needs to save/load.
obj an array of pointers to objects. So when you write obj to disk in the way you're doing it, you're writing the pointer values+some random bits of junk. Reading back from disk reads in some now-dead pointers and some random junk.
Oh, at the risk of being redundant, fread()/fwrite() are evil and shouldn't be touched with a three-metre pole. Your programme and data will at the least be architecture dependent, almost certainly be operating system dependent, probably compiler (and compiler version) dependent and maybe even optimisation switch dependent.
Oh, at the risk of being redundant, fread()/fwrite() are evil and shouldn't be touched with a three-metre pole. Your programme and data will at the least be architecture dependent, almost certainly be operating system dependent, probably compiler (and compiler version) dependent and maybe even optimisation switch dependent.
What do you suggest should be used instead? Suppose I have this:
struct THING { int a; char b; double c; };
and I want to save one of these struct to a file in a way that is completely indepentent of all those things. I feel that this could actually be quite problematic, because sizeof(int) and sizeof(double) might change, and there is the big/little endian thing to worry about as well.
I've only recently started to consider these problems, so if you've got a quick answer then I'd like to hear it.
To save a float/double:
pack_iputl(file, floor(fval)); if(fval-floor(fval)) pack_iputl(file, 1/(fval-floor(fval))); else pack_iputl(file, 0);
And to read:
long whole = pack_igetl(file); long recip = pack_igetl(file); float fval = whole + (recip ? (1.0/(float)recip) : 0.0);
Should work. To save an int, use pack_igetl/pack_iputl. For char, pack_getc/pack_putc.
Ok, thanks.
It seems to me that 'pack_iputf' and 'pack_igetf' would be useful functions to add to Allegro.
It seems to me that 'pack_iputf' and 'pack_igetf' would be useful functions to add to Allegro.
They are Allegro
I meant like this:
double pack_igetf(PACKFILE *file) { long whole = pack_igetl(file); long recip = pack_igetl(file); return whole + (recip ? (1.0/(float)recip) : 0.0); }
If such a thing is already in Allegro, why didn't you just say "use pack_igetf"?
Ohh... pack_putf/pack_getf.. I thought those were l's, not f's.
Sorry.
Right, what a pain in the butt, and as soon as I eradicate one, I find another bug, and another bug, I think I'm going to rewrite it the way you suggest...
Since doubles/floats are defined specifically by IEEE, why arn't they platform/etc dependant?
can't you pack_fwrite(&my_double, 8, fp), or pack_fwrite(&my_float, 4, fp) ?
In another thread someone said, floats/doubles are even in the same endian on macs as they are on pcs because of the way IEEE has defined them. Is that wrong?
Wrong or not, you shouldn't rely on it, lest you compromise platform-independence.
can't you pack_fwrite(&my_double, 8, fp), or pack_fwrite(&my_float, 4, fp) ?
use: sizeof(double)
pack_fwrite(&my_double, sizeof(double), fp)
I thought the sizes were defined by the IEEE also! 
Anyway, I wrote out a couple of functions for writing object *, and actor * s to a packfile, and just looped through calling those functions, it worked out ok, and the compiler warned me anywhere where I was accidently sending a ptr to a ptr instead of just a ptr, etc...
All is well 
The only functions I had to use were pack_iputl, pack_igetl, pack_putc, and pack_getc, which is rather nice. 
edit: NEW PROBLEM.
this is weird, opening an existing file as F_WRITE_PACKED, doesn't seem to let me overwrite the file... how come?
edit: NEVERMIND, the problem was the filename wasn't the same! duh! sorry!
IEEE754 does not define endianness as interpreted in RAM. It only specifies the floating point format and how operations are carried out. It doesn't matter how the host implementation orders and stores the bits, but it does matter how that implementation is presented (interface) to the user.
IEEE754 specifies also single precision (32-bit) and double precision (64-bit). These sizes are defined in the standard. But the IEEE754 standard is not the C++ standard. It doesn't mean that float maps to single precision IEEE754 and double to double precision.
Now, for practical purposes I think it is safe to assume that floats and doubles are IEEE754 format. I haven't seen a machine use a different format. Even if it is true, I don't think it is a problem if you actually define your file format properly. People say "oh, fwrite is not portable because of endianess etc", but that is only half-right. If you specify in your file format that data is saved in IEEE754 little-endian format, then your code with fwrite will work properly on Intel platforms.
Sure, fwriting a block of integers should be fine (modulo endianesse), but you are likely to run into problems when fwriting a struct.
Right cause structs have padding, possible re-ordering, etc, I was only using structs to get it working...
possible re-ordering
Structs don't reorder. But they can (and do) pad.. sometimes differently depending on optimization switches.
Maybe simply write functions (one for each struct) for writing data:
SaveStructA(FILE *file, char *fileName, char *writingMode, struct structA *structA1)
and use
fprintf(...)
for each member:
structA1->member1
...
edit1:
Structs don't reorder. But they can (and do) pad.. sometimes differently depending on optimization switches.
You can compile module that contains writing of structs without optimization and next link them with optimized code.
You can compile module that contains writing of structs without optimization and next link them with optimized code.
What if you use -Os? Still, it can change between major compiler versions, or between compilers.
What if you use -Os? Still, it can change between major compiler versions, or between compilers.
If we are talking about gcc:
-Os means optimize for space rather than speed
-O0 means disable ALL optimizations, this is default switch
-Os means optimize for space rather than speed
Exactly, so it'd stand to reason it could pack structs on capable systems to reduce the amount of space.
Structs don't reorder.
News to me, I thought I heard someone say that they did -- must have been a different language. I dunno. Anyway you always seem fairly knowledgable KC, so I'll take your word for it.
SaveStructA(FILE *file, char *fileName, char *writingMode, struct structA *structA1)
and use
fprintf(...)
for each member:
structA1->member1
Essentially, that's what I did. Except I used allegros packing routines instead of the standard library's.
Some compiler may reorder
struct { short a; long b; short c; };
to
struct { short a; short c; long b; };
under certain circumstance / with certain options given. There should always be a way to prevent them from doing this, though.
That was my understanding also. Is this true?
That was my understanding also. Is this true?
Not as far as I'm aware. AFAIK, and in my experiences, the compiler cannot reorder struct members. Maybe some compilers have an option/extension too, but it shouldn't be the default behavior.
As far as I know, the C standard guarentees the order if struct members. This is so that it is safe to use
struct A { int a; int b; char c; int d; }
and
struct B { int alpha; int beta; char gamma; int delta; }
interchangably. They need not even be declared in the same sourcefile. In fact, it's safe to cast a pointer to
struct C { int i1; int i2; char c1; }
to either of the other two types and use that instead.
If the compiler is allowed to arbitrarily reorder structure members, that would be impossible.
Yes, members of structs are never reordered; polymorphism in C relies on it!
struct BASE_CLASS { int a, b, c; }; struct DERIVED_CLASS { int a, b, c; float extra_data; };
Polymorhism in C is done by type casting to and from stuff that looks like that.
eg.
float my_function(struct BASE_CLASS *foo) { float bar = foo->a * foo->b; if (foo->c == 5) { bar*=((DERIVED_CLASS*)foo)->extra_data; } return bar; }
Erm that doesn't seem relibable. Did I miss something or is the struct automatically padded? And how much padding is it safe to rely on, is there?
| 1 | struct sta |
| 2 | { |
| 3 | int a, b, c; |
| 4 | }; |
| 5 | |
| 6 | struct stb |
| 7 | { |
| 8 | int a, b, c; |
| 9 | int d[5120]; |
| 10 | }; |
| 11 | |
| 12 | void this_shouldnt_work_should_it() //? |
| 13 | { |
| 14 | sta A; |
| 15 | stb * B = (stb)&A; |
| 16 | |
| 17 | B->d[4000] = 15; |
| 18 | } |
How can one be gauranteed that enough padding will be added?
That one definitely shouldn't work. If it compiles then it should segfault if you try writing to the char array
Since doubles/floats are defined specifically by IEEE, why arn't they platform/etc dependant?
In addition to what gillius said, not all computers use IEEE floats.
should segfault
Right, that's what I'd expact.
By those principals -- Why can what Karadoc~~ posted be guaranteed to work?
Karadoc~~'s code is garanteed to work if c==5 only the type of the object is DERIVED_CLASS. It would be more clear if the variable was named 'type'. It's just a matter of design. It's up to the programmer to ensure the types are right.
To perform polymorphism in C you have to keep track of the type of the object anyway.
if (foo->c == 5)
Ahh, I didn't see that before.
Why can what Karadoc~~ posted be guaranteed to work?
You can acces the elements common to sta and stb, but you cannot acces elements specific to stb by casting sta to stb.
Think about it: sta is the base, stb is a derived.
Right that's what I thought, then I saw Karadocs code and I thought he was implying that you could -- I didn't see the c==5 thing....