zoneminder
zoneminder copied to clipboard
Misc fixes
A few fixes I came across and some codeql cleanups.
Damn bro, where did you come from? Lol.
You're really hitting the repo hard with fixes, good job and thank you!
Regarding the: Cast operand to larger destination size to avoid possible overflow is
I kinda hate these random casts. I get why they might be needed, I'm just wondering if there is a nicer way to do it.
For most of them looks like an inline function would probably work, silently promoting the operands. Not sure if it would work on 32bit for the first one, since that's specifically an unsigned long long
.
size_t calculate_image_buffer_size(size_t width, size_t height, size_t colours);
I guess what I am saying is that.... we already made imagesize an unsigned long long, presumably with the idea that that would fix the overflow problem. Your casting here seems to suggest that it doesn't. That we need to upgrade one of the operands to make it do 64bit multiplication before assigning to imagesize. We can be fairly sure for at least a few more years that width and height won't be very large, but we could just make them 64 integers too. I suppose another thought I have is that perhaps we should be using size_t instead of unsigned long long.
Right, the issue (and warning) is that the calculation happens before the promotion.
size_t
should be fine on all platforms. that still handles 8k images on 32bit (4 bytes), and is 8 bytes on 64bit.
Using size_t
for size, width, height, etc. should also fix the issue. If that's your preference, I can look at doing that. A quick grep shows only about 500 lines.
Wouldn't it make sense to use the fixed-width typedefs ([u]int{8,16,32,64}
) at least in those places where we don't want this platform-dependent ambiguity?