darlinghq / darling-dmg

FUSE module for .dmg files (containing an HFS+ filesystem)
http://www.darlinghq.org
GNU General Public License v3.0
273 stars 45 forks source link

uint -> uint32_t. The project doesn't use the type 'uint' #82

Closed tomkoen closed 6 years ago

LubosD commented 6 years ago

I think we're casting to the type used by zlib/libbz2, aren't we?

tomkoen commented 6 years ago

zlib use another type "uInt". bzip2 use "unsigned int" I don't think it's cool to have so many variants of unsigned integer.

jief666 commented 6 years ago

I would argue that cast to uint32_t is not a good idea because it's assuming that int are 32 bits.

As it is now : if compiled on a platform with 64 bits int (for example), there is a loss of precision hidden by the cast.

The safe side is to cast to the unsigned counterpart type of bytesRead. Because the test just before, we know that bytesRead is positive. So cast to unsigned int is safe and, cherry on the cake, doesn't cost (no CPU instruction is needed). Then, we let the compiler convert to the m_strm.avail_in type. If, one day, there would be loss of precision, compiler will tell.

But looking at readSome, we can see that rd is int even though the return type of read is int32_t. I change that in PR #83. Now, cast to uint32_t are safe.

tomkoen commented 6 years ago

int is never 64-bit

jief666 commented 6 years ago

"never" IS an assumption. They tend to be proved wrong, one day. Doesn't harm to be 100% safe instead of coincidentally safe.