2

I have been given a legacy code, where someone(s) have carelessly assigned double values to int variables like:

int a = 10;
double b = 20;
a = b;

Now to get rid of the

warning C4244: '=': conversion from 'double' to 'int', possible loss of data

warnings, I tried editing the code upstream and get rid of the unnecessary double variables, but it turns out to be too messy!

I could also use casting:

a = (int) b;

but actually there is no guarantee that b will be within the limits of an integer. I thought of making a helper function:

int safeD2I(double inputVar){
  if ((inputVar < INT_MAX) && (INT_MIN < inputVar)){
    return (int) inputVar;
  } else {
    exit(-1);
  }
}

but I'm not sure if this is the best implementation. I was wondering if there is a more canonical way to handle this issue?

what I want:

  • in the case the b variable being outside of the integer bounds, the program stops immediately
  • prints an error message to the terminal indicating that the specific line and time when this issue happened.

Thanks for your support in advance.

Foad S. Farimani
  • 12,396
  • 15
  • 78
  • 193
  • 1
    By the way your code would pass _any_ value as it is now. You need `&&` instead of `||`. – CherryDT Mar 22 '20 at 15:16
  • If you're looking for a method to see if there was any data loss, it seems like you just want to do the assignment and then do a comparison. eg `if( (double)a - b > epsilon) ...` – William Pursell Mar 22 '20 at 15:16
  • `a = (int) myDouble` is a sign of a bigger trouble than the warning may indicate. Ideally, you should examine your code to see if you really need the `int` or in some rare cases, the `double`. Perhaps you could use `double a = floor(myDouble)` instead, or manipulate rational numbers with your own implementation instead of going for a double. In any case, learning the line number where the break happened would unlikely be sufficient to figure out the problem. – Sergey Kalinichenko Mar 22 '20 at 15:21
  • It depends on what you mean for safe mode. If the value **can be** greater of `MAX_INT` try to use `long long int` instead of simple `int`. If the value **cannot be larger** simply cast value. The use of `double` for intermediate calculations non necessarily lead to an overflowing `int` result, but the choice may be due to intermediate very small calculations (i.e. coefficients < 1.0). – Frankie_C Mar 22 '20 at 15:22
  • @CherryDT what a silly mistake. thank you :) – Foad S. Farimani Mar 22 '20 at 15:24
  • @dasblinkenlight and @Frankie_C `a` has to be an integer because it is later used for indexing some array. Now whether I can just make all the troubling doubles to ints, or if we really need double, I'm not sure. For now I want to make the code safe, so later I can catch possible issues. – Foad S. Farimani Mar 22 '20 at 15:27
  • 1
    If indexing an array is the only reason for conversion, you are in a much better shape. Make a function that lets you access an array indexed by a `double`, localize all double-to-size_t conversions there, and implement a boundary checker for your specific array, because `MAX_INT` is often too large of an index, even though it fits in an `int` without a problem. – Sergey Kalinichenko Mar 22 '20 at 15:42
  • @dasblinkenlight oh, I hadn't thought of the fact that `a`s must be `unsigned int`s or `size_t`. I will change them. thanks. – Foad S. Farimani Mar 22 '20 at 15:48
  • 1
    Based on what your C proficiency seems to be, I would be **very wary** of making any unnecessary or "cleanup" changes to this code. You are likely to break far more than you fix. Make changes only where you've done analysis to determine that there's an active bug, and don't refactor things to fix. Make simple direct inline fixes. – R.. GitHub STOP HELPING ICE Mar 22 '20 at 15:49
  • @R..GitHubSTOPHELPINGICE thanks for flushing my confidence down :) However, this is not the main repo, but a fork and the main purpose of doing this for me to learn. – Foad S. Farimani Mar 22 '20 at 15:51
  • 1
    See [Can a conversion from double to int be written in portable C](https://stackoverflow.com/questions/51104995/can-a-conversion-from-double-to-int-be-written-in-portable-c). That addresses the range issue while avoiding undefined behavior, but, once you have dealt with that, dealing with the fraction issue is easier. – Eric Postpischil Mar 22 '20 at 15:57
  • @EricPostpischil thank you. That seems like a great read. – Foad S. Farimani Mar 22 '20 at 16:00

2 Answers2

5

First, there's nothing inherently wrong or unsafe with the code as-is. Conversion by assignment is a completely legitimate part of the C language. If this is legacy code where you don't know if it's being done safely or not, you need to do the analysis to determine that.

If you find that you do need to catch possible out-of-bounds values (which produce undefined behavior (!!) when converted to int), your code is both wrong and fragile. Comparisons like:

double x;
...
if (x < INT_MAX) ...

coerce INT_MAX to type double for the comparison. In practice in a world where double is IEEE double and int is 32-bit, this happens to be safe, but it would be unsafe for example if you changed double to float, since a 32-bit INT_MAX is not representable in single-precision float. The value will be rounded, and then the comparison takes place after the rounding.

Now, it turns out you also have an off-by-one error (<= INT_MAX, not < INT_MAX, is what's in-bounds) as well as incorrect logic (|| instead of &&) so the rounding would actually fix part of that. But it's not right to depend on it. Instead you need to manufacture a power of two you can compare against, so it's safe to convert to floating point. For example:

  • if (x < 2.0*(INT_MAX/2+1) && x >= INT_MIN)
  • if (-x > (-INT_MAX-1) && x >= INT_MIN)
  • if (-x > INT_MIN && x >= INT_MIN)

These all assume INT_MIN is actually a power of two (full range twos complement) which is a reasonable real-world assumption and required by C2x+. If you want more generality it's more work.

Finally, I first wrote this as a comment on your question, but the more I think about it, it really belongs in an answer: Based on what your C proficiency seems to be, I would be very wary of making any unnecessary or "cleanup" changes to this code. You are likely to break far more than you fix. Make changes only where you've done analysis to determine that there's an active bug, and don't refactor things or change types to fix. Make simple direct inline fixes.

R.. GitHub STOP HELPING ICE
  • 208,859
  • 35
  • 376
  • 711
  • thanks. it is a bit difficult for me at the moment to comprehend you answer. I will study what you said and will get back here if I had more questions. – Foad S. Farimani Mar 22 '20 at 15:39
  • 2
    Note well, @Foad, that a good practice even for an experienced programmer when tasked with revising existing code without changing its behavior is to prepare automated tests of the code that is to be revised. These should be as thorough as possible, covering normal, extreme, and error cases in detail. The tests can be validated against the original code, and the revised code then validated against the tests. It is neither simple nor easy to do this well, but the result is a valuable addition to the code base even after the immediate code revision task is complete. – John Bollinger Mar 22 '20 at 16:01
  • @JohnBollinger Testing is for sure in the plan. Might probably use Google tests. But for now, I want to make sure I can get rid of the compile warning. and no worries about the code. This is a fork. and I'm working on a branch of the develop branch. so if anything goes wrong I have the master safe. – Foad S. Farimani Mar 22 '20 at 16:07
  • Setting out to get rid of compiler warnings for its own sake, with no attention to what the code is actually doing and whether your changes might be breaking, is a sure recipe for breaking things. That's not how compiler warnings are meant to be used. – R.. GitHub STOP HELPING ICE Mar 22 '20 at 16:45
  • `x >= INT_MIN` fails incorrectly `x` values less than `INT_MIN` by less than 1.0. When `INT_MIN` is 2's complement, `x - INT_MIN > -1.0` covers that. – chux - Reinstate Monica Mar 22 '20 at 19:36
2

what is the safe way to convert double to int?

if ((inputVar < INT_MAX) && (INT_MIN < inputVar)){ fails edge cases.

Wrong edges as it is more like, but not exactly like (inputVar < INT_MAX + 1) && (INT_MIN - 1 < inputVar)

Be wary of code like some_FP < SOME_INT_MAX as SOME_INT_MAX may not convert to the FP value needed due to the integer type may have more precision that the FP one. This is not usually a problem with int, double.


Test if the double is within the range of (INT_MIN-1 .... INT_MAX+1)1. Note () and not [].

If not, error out or handle in some defined way of your choosing.

Assuming typical 2's complement, but not assuming the precision of double exceeds int (more useful to migrate code to float, long long that way), some sample code:

// FP version of INT_MAX + 1.0 
// Avoid direct (INT_MAX + 1.0) as that can have precision woes
#define DBL_INTMAX_P1 ((INT_MAX/2 + 1)*2.0)

int X_int_from_double(double x) {
  // Coded to insure NAN fails the if()
  if (!(x - INT_MIN > -1.0 && x < DBL_INTMAX_P1)) {
    errno = ERANGE;
    fprintf(stderr, "Error in %s,  %.*e too large\n", __func__, DBL_DECIMAL_DIG - 1, x);

    exit(EXIT_FAILURE);
    // or additional code to handle conversion in some specified manner
    // Example: assuming "wrap"
    if (!isfinite(x)) {
      if (!isnan(x)) return 0;
      if (x > 0) return INT_MAX;
      else return INT_MIN; 
    }
    modf(x, &x); // drop fraction
    x = fmod(x, DBL_INTMAX_P1*2);
    if (x >= DBL_INTMAX_P1) x -= DBL_INTMAX_P1*2;
    else if (x < -DBL_INTMAX_P1) x += DBL_INTMAX_P1*2;
  }
  return (int) x;
}

To record the line where this failed, consider a macro to pass the line number.

int X_int_from_double(double x, unsigned);
#define DOUBLE_TO_INT(x) X_int_from_double((x), __LINE__)

1 Example -2,147,483,648.9999... to 2,147,483,647.9999...

chux - Reinstate Monica
  • 143,097
  • 13
  • 135
  • 256