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;
        /* 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);
                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.

[Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!