HiveBrain v1.2.0
Get Started
← Back to all entries
patterncModerate

Safe multiplication of two 64-bit signed integers

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
bitmultiplicationtwosafesignedintegers

Problem

The function below implements safe multiplication of two 64-bit signed integers, preventing overflow from occurring:

// 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?

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.