1

Take for example int a=INT_MAX-1; and int b=INT_MAX-1; and assume that int is 32-bit and a function

int product(int a,int b)
{
    return a*b;
}

Now here the product a*b overflows resulting in undefined behavior from the standard:

If an exceptional condition occurs during the evaluation of an expression (that is, if the result is not mathematically defined or not in the range of representable values for its type), the behavior is undefined.

However if we have instead

int product(int a,int b)
{
    long long x = (long long)a*b;
    return x;
}

Then assuming this answer is correct and applies to long long as well by the standard the result is implementation-defined.

I'm thinking that undefined behavior can cause anything including a crash so it's better to avoid it all costs, hence that the second version is preferable. But I'm not quite sure if my reasoning is okay.

Question: Is second version preferable or is the first one or are they equally preferable?

lurker
  • 56,987
  • 9
  • 69
  • 103
kingW3
  • 439
  • 10
  • 20
  • 2
    Unless you return `long long` from second version and assign the returned value to a `long long` type, you still have overflow. – taskinoor Jun 22 '18 at 10:25
  • 1
    One causes UB and one doesn't ... I would say that they are both bad. Not really sure what sort of answer you are expecting here – M.M Jun 22 '18 at 10:29
  • @taskinoor actually it is out of range assignment in the second case, not overflow (see the answer linked from the question) – M.M Jun 22 '18 at 10:30
  • @M.M right, I was mistaken. I agree to you that both of them are bad. – taskinoor Jun 22 '18 at 10:32
  • You're also assuming something like `sizeof(long long) >= 2*sizeof(int)`, which is not guaranteed. – aschepler Jun 22 '18 at 11:11
  • @aschepler I assumed that int is 32 bits, and long long is at least 64 bits. – kingW3 Jun 22 '18 at 11:16
  • 1
    Guess it depends on the use case. If the use is `total_charge = product(unit_price, items_sold);` and you drop the billions from your invoice, I would rather have a program crash than go bankrupt. :-) – Bo Persson Jun 22 '18 at 11:29

3 Answers3

2

Both of the options are bad because they do not produce the desired result. IMHO it is a moot point trying to rank them in badness order.

My advice would be to fix the function to be well-defined for all use cases.

M.M
  • 138,810
  • 21
  • 208
  • 365
2

If you (the programmer) will never (ever!) pass values to the product() function that will cause undefined behavior, then the first version, why not.
The second version returns the sizeof(int)*CHAR_BIT least significant bits of the result (this is implementation defined behavior) and still may overflow on architectures where LLONG_MAX == INT_MAX. The second version may take ages to execute on a 8-bit processor with real bad support for long long multiplication and maybe you should handle the overflow when converting long long to int with some if (x > INT_MAX) return INT_MAX;, unless you are only really interested in only the least significant bits of the product result.
The preferable version is that, where no undefined behavior exists. If you aren't sure if multiplication a and b will result in undefined behavior or not, you should check if it will and prepare for such a case.

#include <assert.h>
#include <limits.h>

int product(int a, int b)
{
    assert(a < INT_MAX/b && b < INT_MAX/a);
    if (!(a < INT_MAX/b && b < INT_MAX/a)) 
         return INT_MAX;
    return a * b;
}

or in GNUC:

int product(int a, int b) {
   int c;
   if (__builtin_sadd_overflow(a, b, &c)) {
       assert(0);
       return INT_MAX;
   }
   return c;
}
KamilCuk
  • 120,984
  • 8
  • 59
  • 111
  • 1
    A conversion to signed type with result out of range is often done with the least significant bits, but technically the behavior is implementation-defined. – aschepler Jun 22 '18 at 11:14
0

I believe that slightly tweaked second version might be interesting for you:

int product(int a, int b)
{
  long long x = (long long)a * b;
  if (x < INT_MIN || x > INT_MAX)
  {
    fprintf(stderr, "Error in product(): Result out of range of int\n");
    abort();
  }
  return x;
}

This function takes two integers as long ints, computes their product and checks if the result is in range of int. If it is, we can return it from the function without any bad consequences. If it is not, we can print error message and abort, or do exception handling of a different kind.

EDIT 1: But this code stil expects that (long long)a * b does not overflow, which is not guaranteed when i. e. sizeof(long long) == sizeof(int). In such case, an overflow check should be added to make sure this does not happen. The (6.54) Integer Overflow Builtins could be interesting for you if you don't mind using GCC-dependent code. If you want to stay in C without any extensions, there are methods to detect multiplication overflow as well, see this StackOverflow answer: https://stackoverflow.com/a/1815371/1003701

Miroslav Mares
  • 2,292
  • 3
  • 22
  • 27