One of the hallmarks of “bad code” is when someone reinvents the wheel. In many Code SODs, we show code that could be replaced with a one-line call to a built in, standard library.
That’s one of the the advantages to a high-level language operating on modern hardware. Andrew doesn’t live in high-level land. He does embedded systems programming, often on platforms that don’t have conveniences like “standard libraries”, and so they end up reinventing the wheel from time to time.
For example, a third party wrote them some software. Among other things, they needed to implement their own versions of things like trigonometric functions, random functions, square root, and so on- including atoi
- a function for converting arrays of characters to integers. Andrew was called upon to port this software to slightly different hardware.
In the header file, a comment promised that their atoi
implementation was “compatible with ANSI atoi”. The implementation, however…
long atoi(char *s)
{
char *p;
int l;
long m=1, t=0; /* init some vars */
bool negative = FALSE;
/* check to see if its negative */
if(*s == '-') {
negative = TRUE;
s++;
}
/* not negative is it a number */
else if(s[0] < '0' || s[0] > '9')
{
return 0;
}
l = strlen(s);
p = s + l; /* start from end of string */
/* for each character in the string */
do {
p--; /* work backwards */
t += m * (*p - '0'); /* add new value to total, multiplaying by current multiplier (1, 10, 100 etc.) */
m = (m << 3) + (m << 1); /* multiply multiplier by 10 (fast m * 10) */
l--; /* decrement the count */
} while(l);
if(negative)
t = -t; /* negate if we had a - at the beginning */
return t; /* return the total */
}
The comments came from the original source. Now, this ANSI compatible function has a rather nice boundary check to make sure the first character in either a “-” or a digit. Of course, that’s only the first character. Andrew helpfully provided some example calls that might have unexpected behaviors:
atoi("2z") == 94
atoi("-z") == -74 // at least it got the sign right
atoi("") == [memory violation] // usually triggering a hardware interrupt
For bonus points, their versions of sin
and cos
can have drift of up to two degrees, and their tan
implementation is just a macro of sin/cos
, so it has ever larger errors. Their rand
function is biased towards zero, and their sqrt
implementation uses convergence- it successively approximates the correct result. That much is fine, and pretty normal, but a good sqrt
function uses a target epsilon to know when to stop- keep approximating until the error margin shrinks below the point where you care. This particular sqrt
function just used a fixed number of iterations, so while sqrt(10)
might give you 3.1623
, sqrt(10000)
gave you 1215.5
.