11/03/2011

11-03-11 - BoolYouMustCheck

I've got a lot of functions that return error codes (rather than throw or something). The problem with that is that it's very easy to just not check the error code and then you have incorrect code that can possibly break in a nasty way if the error case is hit and not detected.

One way to test this is like this :


class BoolYouMustCheck
{
private:
    bool m_b;
    mutable bool m_checked;

public :

    //BoolYouMustCheck() : m_b(false), m_checked(false) { }
    BoolYouMustCheck(bool b) : m_b(b), m_checked(false) { }
    
    ~BoolYouMustCheck()
    {
        ASSERT( m_checked );
    }
    
    operator bool () const
    {
        m_checked = true;
        return m_b;
    }

};

it's just a proxy for bool which will assert if it is assigned and never read.

So now you can take a function that returns an error condition, for example :


bool func1(int x)
{
    return ( x > 7 );
}

normally you could easily just call func1() and not check the value. But you change it to :

BoolYouMustCheck func2(int x)
{
    return ( x > 7 );
}

(in practice you probably just want to do #define bool BoolYouMustCheck)

Now you get :


{

    int y = clock();

    // asserts:
    func1(y);

    // asserts :
    bool b1 = func1(y);
    
    // okay :
    bool b2 = func1(y);
    if ( b2 )
        y++;
    
    // okay :
    if ( func1(y) )
        y++;
        
    return y;
}

which is kind of nice.

The only ugly thing is that the assert can be rather far removed from the line of code that caused the problem. In the first case (just calling func1 and doing nothing with the return value), you get an assert right away, because the returned class is destructed right away. But in the second case where you assign to b1, you don't get the assert until the end of function scope. I guess you could fix that by taking a stack trace in the constructor.

(note : if you want to intentionally ignore the return value b1 you can just add a line like (int) b1; to surpress the assert.

5 comments:

johnb said...

GCC has __attribute__((warn_unused_result))

http://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html

MSVC code analysis has the MustCheck property

http://msdn.microsoft.com/en-us/library/ms182042.aspx

Of course, you have to be using those tools, and I think MSVC /analyze is only in VS Enterprise (?), and people will probably be stupid and ignore the warnings or turn them off because there are so many of them.

cbloom said...

I hear good things about /analyze but it's a pretty big commitment.

Unknown said...
This comment has been removed by the author.
gpakosz said...

makes me think of Java checked exceptions and that's not good memory

typopl said...

I used to work with mikesart.

You could use my typo.pl perl script (search CPAN) to statically check for unchecked function return values. There's a hurdle setting up the script, and you'd have to update the script when you add new functions that have MUST_CHECK return values.

old rants