0

This is my minimal reproducible example.

struct Cell
{
};

void initializeCells(unsigned int width, unsigned int height, unsigned int size, Cell* outcell)
{
    outcell = new Cell[(width / size) * (height / size)];
}

int main(int argc, char* argv[])
{
    Cell* cells = nullptr;
    initializeCells(1200, 600, 10, cells);
}

I am getting a warning in line outcell = new Cell[(width / size) * (height / size)]; saying

Warning C26451 Arithmetic overflow: Using operator '*' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '*' to avoid overflow (io.2).

How can I cast the value to a wider type like the warning is suggesting? Also something else that's weird is that the warning sometimes disappears for a few minutes and comes back again and is inconsistent. I am using visual studio 2019 and the inbuilt compiler.

Improve this question
3
  • Thanks to all the answers and my issue is solved. However, when I use size_t and hover my mouse over it, it says unsigned long long. Isn't that a complete overkill to store an int of 1200 and 600? Commented Jan 3, 2021 at 19:41
  • 1
    You aren't really storing anything for very long. And the conversion will potentially be done anyway when the new[] operator is called. Commented Jan 3, 2021 at 19:50
  • 1
    Also, by changing the parameters to size_t you'll likely just be using 3 64-bit registers rather than 3 32-bit registers; and there's nothing wrong with passing unsigned int types - they'll be silently promoted to size_t. Commented Jan 3, 2021 at 20:06

5 Answers 5

Reset to default
1

This creates a size_t from an unsigned int, then multiplies the size_t with an unsigned int.

outcell = new Cell[size_t{ width / size } * (height / size)];

Depending on the use of width, height and size you might instead consider making these a size_t.

Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

1

The size of an array is std​::​size_­t per [dcl.array]/1. Knowing that, you can make that the type of your function parameters like

void initializeCells(std​::​size_­t width, std​::​size_­t height, std​::​size_­t size, Cell* outcell)
{
    outcell = new Cell[(width / size) * (height / size)];
}

Alternatively, you can cast one of the operands to a std​::​size_­t to force the expression to use that as its result type like

outcell = new Cell[(static_cast<std::size_t>(width) / size) * (height / size)];
Improve this answer

Comments

0

The warning is caused by the fact that the operand of new[] is of size_t type, which is apparently a 64-bit unsigned integer on your platform (but the unsigned int arguments are 32-bits).

A quick way to silence the warning is to cast the results of the divisions to size_t type:

    outcell = new Cell[static_cast<size_t>(width / size) * static_cast<size_t>(height / size)];

However, a (perhaps) better way would be to change the argument types of initializeCells to size_t:

void initializeCells(size_t width, size_t height, size_t size, Cell* outcell)
{
    outcell = new Cell[(width / size) * (height / size)];
}
Improve this answer

8 Comments

On MSVC this would trigger the code analysis warning that you are using a C style cast (size_t(...)).
@WernerHenze Actually, it doesn't - it's a functional-style cast. But you're right: I have edited the code sample to use a better (C++) cast.
static_cast is not necessary. MSVC also emits code analysis warnings when using static_cast for arithmetic covnersion (as done here) and suggest using a brace initialization as in size_t{...}.
@WernerHenze I beg to differ. I compiled the posted code snippets with VS-2019 (64-bit build), with full warnings enabled, and there is no warning given. And why is brace initialization any different (really) from my original function-style cast (which actually is a constructor)?
These are not regular compiler warnings. These are code analysis warnings (when enabling code analysis with "MS Native Recommended Rules").
|
0

width, height and size are int which is 4 bytes in size. When multiplied, by default it is cased to a int type.
But the new[] operator looks something like this,

void* operator new[](size_t size) { ... }

This means that the new[] operator takes the size in bytes as a size_t value (which is basically an unsigned 64 bit integer).

To resolve this, you can either static cast the variables before multiplying it like this,

outcell = new Cell[(static_cast<size_t>(width) / static_cast<size_t>(size)) * (static_cast<size_t>(height) / static_cast<size_t>(size))];

Or you can alter the function signature like this,

// Note that size_t is unsigned long long under the hood.
void initializeCells(size_t width, size_t height, size_t size, Cell* outcell)
Improve this answer

1 Comment

"This might be an issue" It isn't. "as the size of it is 0 bytes." It isn't.
0

This warning occurs because if width/size ends up being the max value of unsigned int and height/size is too, the multiplication would overflow. This is relevant because allocating an array always takes an integer with the size of the system's address size (8 bytes in your case, same size as the type size_t has). By simply casting your values to size_t, you can mitigate this warning:

#include <cstddef> // this header contains the size_t type

// ...

outcell = new Cell[static_cast<size_t>(width / size) * static_cast<size_t>(height / size)];
Improve this answer

1 Comment

You are using two C style casts. This is not modern C++ (compare CppCoreGuidelines).

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.