| ▲ | unwind 4 hours ago | |
More obvious problems with the code as shown: - The macros doing arithmetic (KiB, MIN etc) don't put their arguments in parentheses, that is very dangerous. - The code assumes that int and i32 are the same size. - Don't cast the return value of malloc(). Please Google this. I appreciated the use of structs by value, that's a nice design. I agree there's a risk of overflow in the arithmetic around sizes/alignment, but with a 64-bit `usize` that should be mostly theoretical. | ||
| ▲ | flohofwoe 3 hours ago | parent [-] | |
> The code assumes that int and i32 are the same size. Tbf this is extreme nitpicking. It's very unlikely that the code will ever be built on a compiler where int is not 32 bits. Replacing the sizeof(int) with sizeof(i32) is better of course, and tbh it looks like an oversight, not intentional. > Don't cast the return value of malloc(). Please Google this. This is required when the code should also build in C++ mode (which some weirdos seem to prefer for "improved type safety") - in any case, the explicit cast is also entirely valid C code, so not wrong just redundant. > The macros doing arithmetic (KiB, MIN etc) don't put their arguments in parentheses, that is very dangerous. Agreed. I would add though: > Don't typedef C primitive types to your own shortcuts (like int32_t => i32), at least not when those typedefs leak into the public API, because then your i32 may collide with another library's i32. | ||