patterncMinor
Test if arithmetic operation will cause undefined behavior
Viewed 0 times
behaviorundefinedoperationwilltestcausearithmetic
Problem
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
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 && 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 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);
}Solution
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.
This hints that using a macro or enumerated result would be less error prone than
As @JS1 noted, replace
[Edit]
Candidate
[Edit2]
A potential simplification with
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 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 INT_MAX - a);
}
int is_undefined_sub1(int a, int b) {
return (b INT_MAX + b) : (a < INT_MIN + b);
}Code Snippets
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.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;
}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
}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);
}Context
StackExchange Code Review Q#93687, answer score: 6
Revisions (0)
No revisions yet.