Eliminating inherited overlong MACRO

Question

I have inherited a very long set of macros from some C algorithm code.They basically call free on a number of structures as the function exits either abnormally or normally. I would like to replace these with something more debuggable and readable. A snippet is shown below

#define FREE_ALL_VECS {FREE_VEC_COND(kernel);FREE_VEC_COND(cirradCS); FREE_VEC_COND(pixAccum).....
#define FREE_ALL_2D_MATS  {FREE_2D_MAT_COND(circenCS); FREE_2D_MAT_COND(cirradCS_2); } 
#define FREE_ALL_IMAGES {immFreeImg(&imgC); immFreeImg(&smal.....
#define COND_FREE_ALLOC_VARS {FREE_ALL_VECS FREE_ALL_2D_MATS FREE_ALL_IMAGES}

What approach would be best? Should I just leave well alone if it works? This macro set is called twelve times in one function. I'm on Linux with gcc.

Answer 1

Usually I refactor such macros to functions, using inline functions when the code is really performance critical. Also I try to move allocation, deallocation and clean up stuff into C++ objects, to get advantage of the automatic destruction.

Answer 2

Ideally, I would use inline functions instead of using macros to eliminate function call overhead. However, basing from your snippet, the macros you have would call several nested functions. Inlining them may not have any effect, thus I would just suggest to refactor them into functions to make them more readable and maintainable. Inlining improves performance only if the function to be inlined is simple (e.g. accessors, mutators, no loops).

Answer 3

I believe this is your decision. If the macros are creating problems when debugging, I believe it is best to create some functions that do the same things as the macros. In general you should avoid complicated macros. By complicated I mean macros that do something more than a simple value definition.

Recommended:

// it is best to use only this type of macro
#define MAX_VALUE 200

The rest is not recommended (see example below):

// this is not recommended

#define min(x,y) ( (x)<(y) ? (x) : (y) )

// imagine using min with some function arguments like this:
//
// val = min(func1(), func2())
//
// this means that one of functions is called twice which is generally
// not very good for performance

Answer 4

If they are broken then fix them by converting to functions.

If they're aren't broken then leave them be.

If you are determined to change them, write unit-tests to check you don't inadvertently break something.