4
\$\begingroup\$

I've written a command line calculator as an exercise to get ready for my upcoming first programming job. The whole program source is around 450 lines of code, which I think is too long for a single question. I'll therefore post the code in parts starting with this one.

Any software needs to validate it's data. The functions below are used to test whether a basic arithmetic operation on two signed integers a and b will cause undefined behavior or not. They are checking if the result would be out of bounds and for division by zero. If they return zero the operation is then carried out and the result returned. If they return non-zero, an error message is printed out to the user. The logic of these functions is taken from this page of the SEI CERT C Coding Standard.

My main concern with this code is the is_undefined_mult function, which I don't think is very readable.

int is_undefined_add(int a, int b)
{
    return (a > 0 && b > INT_MAX - a) ||
           (a < 0 && b < INT_MAX - a);
}

int is_undefined_sub(int a, int b)
{
    return (b > 0 && a < INT_MAX + b) ||
           (b < 0 && a > INT_MAX + b);
}

int is_undefined_mult(int a, int b)
{
    if (a > 0) {
        if (b > 0) {
            if (a > INT_MAX / b) {
                return 1;
            }
        }
        else {
            if (b < INT_MIN / a) {
                return 1;
            }
        }
    }
    else {
        if (b > 0) {
            if (a < INT_MIN / b) {
                return 1;
            }
        }
        else {
            if (a != 0 && b < INT_MAX / a) {
                return 1;
            }
        }
    }

    return 0;
}

int is_undefined_div(int a, int b)
{
    return b == 0 || (a == INT_MIN && b == -1);
}
\$\endgroup\$
  • \$\begingroup\$ You can chain two or more conditions using &&. \$\endgroup\$ – Spikatrix Jun 16 '15 at 5:00
6
\$\begingroup\$

OP edited post.
Is the description backwards?

If they return non-zero the operation is then carried out and the result returned. If they return zero, an error message is printed out to the user.

int is_undefined_mult(int a, int b) {
    if (a > 0) {
        if (b > 0) {
            if (a >= INT_MAX / b) {
                return 1;  // 1 is the overflow condition here.

This hints that using a macro or enumerated result would be less error prone than 0 or 1.


a == INT_MIN && b == -1 is only a problem with 2's complement:

int is_undefined_div(int a, int b) {
  #if INT_MIN < -INT_MAX 
    if (a == INT_MIN && b == -1) return 1;
  #endif
  return b == 0;
}

As @JS1 noted, replace INT_MAX with INT_MIN in 2 places. Otherwise, per my tests, the functions are correct, expecting is_undefined_div().


[Edit]
Candidate is_undefined_mult() simplification - really just a collapsing of the if structure.

int is_undefined_mult1(int a, int b) {
  if (a > 0) {
    if (b > 0) {
      return a > INT_MAX / b;       // a positive, b positive
    }
    return b < INT_MIN / a;         // a positive, b not positive
  }
  if (b > 0) {
    return a < INT_MIN / b;         // a not positive, b positive
  }
  return a != 0 && b < INT_MAX / a; // a not positive, b not positive
}

[Edit2]
A potential simplification with is_undefined_add()/is_undefined_sub().

int is_undefined_add1(int a, int b) {
  return (a < 0) ? (b < INT_MIN - a) : (b > INT_MAX - a);
}

int is_undefined_sub1(int a, int b) {
  return (b < 0) ? (a > INT_MAX + b) : (a < INT_MIN + b);
}
\$\endgroup\$
5
\$\begingroup\$

Typos?

Your first two functions appear to have typos:

int is_undefined_add(int a, int b)
{
    return (a > 0 && b > INT_MAX - a) ||
           (a < 0 && b < INT_MIN - a);   // <-- Was MAX
}

int is_undefined_sub(int a, int b)
{
    return (b > 0 && a < INT_MIN + b) ||  // <-- Was MAX
           (b < 0 && a > INT_MAX + b);
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.