daokoder / dao-modules

Dao Standard Modules
http://daovm.net
12 stars 5 forks source link

binary/dao_binary.c: ISO C width of numbers not enforced #50

Closed dumblob closed 9 years ago

dumblob commented 9 years ago

DaoDecoder_ReadI8() functions are not reliable on all platforms Dao supports. In POSIX, unsigned char is required to be 8 bits wide (unlike in ISO C). But everything else is according to ISO C, which just says, that short should be at least 16 bit, long at least 32 bit etc.

In the DaoBinary_OnLoad() function, only int is checked for size, but nothing else. But all types used in DaoDecoder_ReadX functions should be checked. Or we can use uint8_t uint16_t uint32_t and uint64_t from to enforce the widths (and for float and double assume they're 32b or 64b wide). I wouldn't rather use long double, because it's very different among platforms.

Night-walker commented 9 years ago

Thanks, I used the stuff from stdint.h for integers and added warnings for floating point values.

dumblob commented 9 years ago

Good. It seems all the reasons for the check for 32b int in DaoBinary_OnLoad() are mitigated and if I'm not missing anything, the check might be removed.

By the way, the functions DaoBinary_Pack() and DaoBinary_Unpack() support only 32b int - is there a reason for not supporting 64b int?

Night-walker commented 9 years ago

Good. It seems all the reasons for the check for 32b int in DaoBinary_OnLoad() are mitigated and if I'm not missing anything, the check might be removed.

The module's functions won't work properly if Dao int is 32-bit -- reading/writing 64-bit values will likely result in data loss.

By the way, the functions DaoBinary_Pack() and DaoBinary_Unpack() support only 32b int - is there a reason for not supporting 64b int?

64-bit int doesn't require any packing/unpacking because bytes in the file and the array will be 1:1 in this case :)

dumblob commented 9 years ago

The module's functions won't work properly if Dao int is 32-bit -- reading/writing 64-bit values will likely result in data loss.

Right, I didn't realize, that the check is for dao_integer and not for int. Shame on me.

64-bit int doesn't require any packing/unpacking because bytes in the file and the array will be 1:1 in this case :)

It seems, I'm really missing something, because both pack() and unpack() have mandatory argument size: enum<byte,word,dword>, but there is no qword and this in conjunction with the code

    size_t sizes[] = {1, 2, 4};
    size_t size =  sizes[p[2]->xEnum.value];

in functions DaoBinary_Pack() and DaoBinary_Unpack() makes it IMHO impossible to put 64b int into the destination array.

dumblob commented 9 years ago

Maybe, you meant arr = (array<int>) io_stream, but I don't think it works. Will try in a second.

dumblob commented 9 years ago

Nope, this casting simply doesn't work (not arr = (array<int>)"my string" and not even simple x={1,2,3}; y=(array<int>)x).

Night-walker commented 9 years ago

There are bin.read() and bin.write() exactly to handle an array<int> without packing/unpacking.

dumblob commented 9 years ago

Ok, thanks.