Finding statement pattern in c++ file

Question

I have a macro that looks like this:

#define coutError    if (VERBOSITY_SETTING >= VERBOSITY_ERROR)    ods()

where ods() is a class that behaves similarly to cout, and VERBOSITY_SETTING is a global variable. There are a few of these for different verbosity settings, and it allows the code to look something like this:

if (someErrorCondition)
{
    // ... do things relating to the error condition ...

    coutError << "Error condition occurred";
}

And there is functionality in this framework to set the verbosity, etc. However, the obvious pattern breaks when not using braces in something like this:

void LightSwitch::TurnOn()
{
    if (!PowerToSwitch)
        coutError << "No power!";
    else
        SwitchOn = true;
}

because of the macro, will turn into this:

void LightSwitch::TurnOn()
{
    if (!PowerToSwitch)
        if (VERBOSITY_SETTING >= VERBOSITY_ERROR)
            ods() << "No power!";
        else
            SwitchOn = true;
}

Which is not the intended functionality of the if statement.

Now, I understand a way to fix this macro properly so it doesn't cause this problem, but I'd like to run an audit on the code and find any place that has this pattern of "if (...) coutError << ...; else" to find out if there are any other cases where this happens to make sure that when fixing the macro, it will indeed be correct functionality.

I can use any language/tool to find this, I just want to know the best way of doing that.

Answer 1

I think the real problem is the use of the macro - not the code that gets preprocessed. But I don't think this is the answer you are looking for.

I would find a way to do it without using macros at all - and if you do use conditional compiles you can do so in the call to ods() - depending on some #define it can use whatever functionality you want.

just my $.02

Answer 2

regular expression search?

Answer 3

You could try - temporarily - modifying the macro to something like this, and see what doesn't compile...

#define coutError {} if (VERBOSITY_SETTING >= VERBOSITY_ERROR) ods()

The 'else' clauses should now give errors.

Answer 4

I think all good usages of the macro are preceeded with '{' or ';'.

So try this regular expression:

[^{;]\s*coutError

You'll need to turn on multi-line matching and search against entire files.

You might need to grab more stuff so that you can locate the line in question :-)

Alternatively changing the macro is a neat idea, if we can work out something that will fail correctly. Maybe a block followed by the conditional operator:

#define coutError {} (VERBOSITY_SETTING >= VERBOSITY_ERROR)?(ods()):(nullstream())

(But does require implementing a nullstream() operator.)

(Alternatively get rid of the conditional entirely temporarily - as you suggest is a comment to another answer @Roddy (currently the selected answer)).

p.s. I know you didn't ask, but an easy way to wrap the macro to make it safe is with a do {} while(false) loop.

Answer 5

I see in your comments above that you're thinking of using a template in the macro, and I can't comment yet (I'm 9 points short of it), so...

What stops you from doing

#define CoutError(s) { if (VERBOSITY_SETTING >= VERBOSITY_ERROR){ ods(s); } }

And then

void LightSwitch::TurnOn()
{
    if (!PowerToSwitch)
        CoutError("No power!");
    else
        SwitchOn = true;
}

And redefine ods to admit a string, or alternatively if you can't, then just define an OdsHelper function which takes a string and whose body is simply ods << inString?

I wouldn't play with macros trying to mimic the << syntax if there is no clear gain. At least with macros simulating the function syntax we're more used and we know we have to write the blocks to prevent strange issues.

Do you really need the << syntax?

And do you really need to introduce a template for this simple behaviour?

Oh and one last thing - don't use macros.

Answer 6

Don't bother trying to find all of the locations in your code where a logic error is occurring -- fix the problem at its source! Change the macro so that there's no possibility of error:

#define coutError if(VERBOSITY_SETTING < VERBOSITY_ERROR); else ods()

Note that what I've done here is inverted the test, added an empty statement for the then clause, and put the output object in the else clause. This still allows you to use << foo << bar after the macro, and if you have a trailing else clause belonging to a different if statement, it will get matched up properly, since it expands like so:

if(foo)
    coutError << bar;
else
    baz();

becomes

if(foo)
    if(VERBOSITY_SETTING < VERBOSITY_ERROR)
        ;
    else
        ods() << bar;
else
    baz();