patterncModerate
Safe multiplication of two 64-bit signed integers
Viewed 0 times
bitmultiplicationtwosafesignedintegers
Problem
The function below implements safe multiplication of two 64-bit signed integers, preventing overflow from occurring:
Did I get the logic for detecting overflows right? I am most interested in making sure that no undefined behavior will occur, and that the function will always return the right results.
We can assume that
For the convenience of the code reviewers (you), I made a complete, self-contained test suite that you can just run in your favorite C/C++ environment:
https://gist.github.com/DavidEGrayson/06cf7ea73f82a05490ba
I am not too interested in code style issues. I am aware it might be nice to assert that the output pointer is not null, and it might be nice to add braces and parentheses. But really I want to know if the code is actually correct.
I am somewhat interested in making the code simpler if you have a way to do that; maybe some of the cases could be combined.
I am also aware that the logic might be easier if I just used a 128-bit integer to store the multiplication result, but I would like to get it right without doing that.
The reason I am asking this is because I am working on adding intsafe.h to mingw-w64 (a GCC compiler for Windows), so I am implementing safe multiplication of signed and unsigned numbers. This function is analogous to
// Multiplies two 64-bit signed ints if possible.
// Returns 0 on success, and puts the product of x and y into the result.
// Returns 1 if there was an overflow.
int int64_mult(int64_t x, int64_t y, int64_t * result)
{
*result = 0;
if (x > 0 && y > 0 && x > INT64_MAX / y) return 1;
if (x 0 && x 0 && y INT64_MAX / -y))
return 1;
*result = x * y;
return 0;
}Did I get the logic for detecting overflows right? I am most interested in making sure that no undefined behavior will occur, and that the function will always return the right results.
We can assume that
int64_t will be a two's complement integer, because this is for GCC and GCC does not support any other type of integer.For the convenience of the code reviewers (you), I made a complete, self-contained test suite that you can just run in your favorite C/C++ environment:
https://gist.github.com/DavidEGrayson/06cf7ea73f82a05490ba
I am not too interested in code style issues. I am aware it might be nice to assert that the output pointer is not null, and it might be nice to add braces and parentheses. But really I want to know if the code is actually correct.
I am somewhat interested in making the code simpler if you have a way to do that; maybe some of the cases could be combined.
I am also aware that the logic might be easier if I just used a 128-bit integer to store the multiplication result, but I would like to get it right without doing that.
The reason I am asking this is because I am working on adding intsafe.h to mingw-w64 (a GCC compiler for Windows), so I am implementing safe multiplication of signed and unsigned numbers. This function is analogous to
LongLongMult in intsafe.h. Your contribution will help make this header safer. You can see my progress here.Solution
Given that you are writing for gcc, is there a reason you're not using the builtins?
But as to your question, it looks correct to me, modulo the answer by the other poster.
bool __builtin_add_overflow (type1 a, type2 b, type3 *res)
bool __builtin_sub_overflow (type1 a, type2 b, type3 *res)
bool __builtin_mul_overflow (type1 a, type2 b, type3 *res)But as to your question, it looks correct to me, modulo the answer by the other poster.
Code Snippets
bool __builtin_add_overflow (type1 a, type2 b, type3 *res)
bool __builtin_sub_overflow (type1 a, type2 b, type3 *res)
bool __builtin_mul_overflow (type1 a, type2 b, type3 *res)Context
StackExchange Code Review Q#98791, answer score: 12
Revisions (0)
No revisions yet.