How do I mark code with side effects?

Question

I'm working on a project on an 8051 where every byte counts. As such, I am using some global variables where I normally wouldn't. The normal method of passing pointers into a function adds too much overhead here.

I have a number of functions that use single bit variables (a compiler specific extension to C) to signal the outcome of a function in addition to the normal return value.

bit global_error_flag = 0;
bit global_data_ready_flag = 0;

unsigned char A_Function (void) {
    // Do some stuff

    if ( badness ) {
        global_error_flag = 0;
        global_data_ready_flag = 1;

        return 0;
    }

    if ( data_is_ready_use ) {
        global_data_ready_flag = 1;
    }

    return a_value;    
}

void Other_Function (void) {
    unsigned char c;

    c = A_Function();

    if( global_error_flag) {
        // Do error stuff here.
    }
    else
    if( global_data_ready_flag ) {
        // Do data processing here.
    }
    global_error_flag = 0;
    global_data_ready_flag = 0;

}

Given that the technique is evil, is there some way I can make the code clearer?

How best should I indicate which function calls will have the side-effect of setting these flags? Are comments enough? Should I name the functions to indicate their API (quasi-hungarian-style)? Should I use a macro to mark such calls:

#define FUNCTION_SETS_FLAGS(code) (code)

FUNCTION_SETS_FLAGS( c = A_Function() );

Any other ideas?

Answer 1

Your globals are labelled for clarity, that's a good start.

Ideally you want something that won't compile if you get it wrong. That means macros and comments won't work.

I'd stick to a naming convention for the functions - not necessarily Hungarian, but something like A_Function_Returns_Flags, or less verbose if you can think of that.

Answer 2

Using a convention, whether you want to call it "Hungarian" or not, is the best way I can think to mark this offhand. Stylistically, some sort of naming prefix would be preferable over the empty #define, at least to me.

This is actually pretty common, I think. I know that the S60 programming environment uses a lot of conventional tags on functions to indicate that they throw exceptions, for example.

Answer 3

I did my Ph.D. on a similar issue in Java. I can tell you the one thing you shouldn't do: don't rely on the documentation because then you depend on someone actually reading it. You need to add some hint in the method name to indicate that the user should read the docs to learn about side effects. If you pick something and are consistent with it, you probably stand the most chance.

Answer 4

If you just want to mention that a function affects global variable(s), then a simple (Hungarian) prefix might help.

But if you want to mention every single flag(s) that it affects, then, using the function header is probably the way to go. Like for example,

  /*************************************************************************
     * FUNCTION    : <function_name>
     * DESCRIPTION : <function description> 
     * PARAMETERS  : 
     *  Param1  - <Parameter-1 explanation>
     *  Param2  - <Parameter-2 explanation>
     *  Param3  - <Parameter-3 explanation>
     * RETURN      : <Return value and type>
     * GLOBAL VARIABLES USED: 
     *  Global1 - <Global-1 explanation>
     *  Global2 - <Global-2 explanation>
     *  Global3 - <Global-3 explanation> 
  *************************************************************************/

Answer 5

This doesn't really help you, but GCC has a way to do the opposite of what you want: to mark functions which have no side effects. See the const and pure attributes. This is more for optimization than documentation, thought: if the compiler knows that a given function does not examine any data other than its arguments, it can perform smarter optimizations such as loop-invariant code motion.

Answer 6

You could use a macro to simulate the function to have more parameters:

unsigned char _a_function(void);

#define A_Function(ret_val) (*(ret_val) = _a_function(), !global_error_flag)

...
unsigned char var;
/* call the function */
if (!A_Function(&var))
{
    /* error! */
}
else
{
    /* use var */
    var++;
}

I haven't tried to compile it, so cannot say that this will work, but I think it should.

Answer 7

First I would try to code it in a way that there are only one producer and only one consumer for each of those flags. Then I would clear/set a flag only when needed. As for indicating the side-effect, A standard header on top of the function, doxygen style, should be enough:

    // Function func
    // Does something
    // Consumes ready_flag and  sets error_flag on error.

    int func()
    {
        if (ready_flag)
        {
            //do something then clear the flag
            if (some_error)
                error_flag = x;
            ready_flag = 0;
        }
        //don't mess with the flags outside of their 'scope'
        return 0;
    }

On the other hand, if the error and ready flags are mutually exclusive, you could use a byte (or bits inside a byte/register) to indicate readiness or an error state.

0 for an error, 1 for not-ready/error-free and 2 for ready/error-free (or -1, 0, 1, whatever)

IIRC, the standard 8051 instruction set doesn't operate on single bits, so using a whole byte for (various) flags shouldn't give you a huge performance hit.

Answer 8

If you haven't done so already, you might also want to check out the sdcc project on sourceforge, it's a C compiler specifically meant to be used for embedded development which also targets the 8051, in addition the compiler supports a number of custom, target-specific and non-standard compiler intrinsics for various use cases, also I have personally found the development team to be very open to and responsive about ideas for new enhancements and other related feature requests.

Answer 9

If you really have to stick with these global variables, you can make it obvious that a function may modify them by expecting references to them as function arguments:

unsigned char A_Function (bit *p_error_flag, bit *p_data_ready_flag)
{
  ...
}