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);
}
asked Jun 15, 2015 at 21:52
\$\endgroup\$
1
  • \$\begingroup\$ You can chain two or more conditions using &&. \$\endgroup\$ Commented Jun 16, 2015 at 5:00

2 Answers 2

7
\$\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);
}
answered Jun 16, 2015 at 0:50
\$\endgroup\$
0
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);
}
answered Jun 15, 2015 at 23:36
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.