I consider code cleanup to be important, almost as important as actually writing code, especially since I spend a lot of time doing maintenance and support. During idle time, I enjoy digging through existing code to find overcomplicated algorithms and dead code and see what I can simplify or even delete. While there may not be an immediate benefit, it makes future debugging far easier if you trim out dead code and shorten 3,000 line C files to 500 line ones. And quite often, subtle and difficult-to-diagnose bugs simply go away.

While doing this on one of our products, I found a very old public API that appears to have never been completed. At least I hope so, because there’s no way any of it does anything remotely useful.

Here are a couple representative functions. No, I don’t know what they’re supposed to do. 

    unsigned long error_query(HardwareHandle handle, long* errorCode, char errorMessage[])
    {
        *errorCode = 0;
        strcpy(errorMessage, "error_query not Implemented");
        return API_SUCCESS;
    }

    unsigned long revision_query(HardwareHandle handle, char revision[])
    {
        strcpy(revision, "Unknown");
        return API_SUCCESS;
    }

There are quite a few more functions just like this, but you get the point.

And then there’s this beauty. To provide some background, we manufacture PCI devices for our industry. They have an FPGA loaded with firmware to do some useful stuff and a C API to communicate with it. One of the firmware functions is an internal self-test which is useful for verifying that new firmware versions function or that hardware isn’t physically damaged. The API can poke a register to trigger the test and then monitor another register to see when it finishes and get the result. But here’s how this API’s version of self-test is–and always has been–implemented: 

    void self_test(HardwareHandle handle, int* testResult, char result[])
    {
        HardwareDevice* device = (HardwareDevice*)handle;
        unsigned long retval = API_SUCCESS;
        *testResult = 0;
        // retval = device->doSelfTest();
        if (retval)
        {
            strcpy(result, "Self Test Successful");
        }
    }

How embarrassing.

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