globals, re-entrant code and thread safety

Question

I'm trying to decide whether to implement certain operations as macros or as functions.

Say, just as an example, I have the following code in a header file:

extern int x_GLOB;
extern int y_GLOB;

#define min(x,y) ((x_GLOB = (x)) < (y_GLOB = (y))? x_GLOB : y_GLOB)

the intent is to have each single argument computed only once (min(x++,y++) would not cause any problem here).

The question is: do the two global variables pose any issue in terms of code re-entrancy or thread safeness?

I would say no but I'm not sure.

And what about:

#define min(x,y) ((x_GLOB = (x)), \
                  (y_GLOB = (y)), \
                  ((x_GLOB < y_GLOB) ? x_GLOB : y_GLOB)

would it be a differnt case?

Please note that the question is not "in general" but is related to the fact that those global variables are used just within that macro and for the evaluation of a single expression.

Anyway, looking at the answers below, I think I can summarize as follows:

• It's not thread safe as nothing guarantees that a thread is suspended "in the middle" of the evaluation of an expression (as I hoped instead)

• The "state" those globals represent is, at least, the internal state of the "min" operation and preserving that state would, again at least, require to impose restrictions on how the function can be called (e.g. avoid min(min(1,2),min(3,1)).

I don't think using non-portable constructs is a good idea either so I guess the only option is to stay on the safe side an implement those cases as regular functions.

Answer 1

The globals are not thread-safe. You can make them so by using thread-local storage.

Personally, I'd use an inline function instead of a macro to avoid the problem altogether!

Answer 2

Generally speaking, anytime you use global variables, your subroutine (or macro) is not re-entrant.

Anytime two threads access the same data or shared resource, you have to use synchronization primitives (mutexes, semaphores, etc) to coordinate access to the "critical section".

See Wikipedia reentrant page

Answer 3

That code is not safe for multithreading.

Just avoid macros, unless indispensable ("Efficient C++" books have examples of those cases).

Answer 4

Silently modifying global variables inside a macro like min is a very bad idea. You're much better off

• accepting that min can evaluate its argument multiple times (in which case I'd call it MIN to make it look less like a regular C function), or
• write a regular or inline function for min, or
• use gcc extensions to make the macro safe.

If you do any of these things, you eliminate the global variables entirely and hence any questions about reentracy and multi-threading.

Option 1:

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

Option 2:

int min(int x, int y) { return x < y ? x : y; }

Of course, this has the problem that you can't use this version of min for different argument types.

Option 3:

#define min(x, y) ({ \
    typeof(x) x__ = (x); \
    typeof(y) y__ = (y); \
    x__ < y__ ? x__ : y__; \
})

Answer 5

It's not thread-safe. To demonstrate this, suppose that thread A calls min(1,2) and threadB calls min(3,4). Suppose that thread A is running first, and is interrupted by the scheduler right at the question mark of the macro, because its time slice has expired.

Then x_GLOB is 1, and y_GLOB is 2.

Now suppose threadB runs for a while, and completes the contents of the macro:

x_GLOB is 3, y_GLOB is 4.

Now suppose threadA resumes. It will return x_GLOB (3) instead of the right answer (1).

Obviously I've simplified things a bit - being interrupted "at the question mark" is pretty handwavy, and threadA doesn't necessarily return 3, on some compilers with some optimisation levels it might keep the values in registers and not read back from x_GLOB at all. So the code emitted might happen to work with that compiler and options. But hopefully my example makes the point - you can't be sure.

I recommend you write a static inline function. If you're trying to be very portable, do it like this:

#define STATIC_INLINE static

STATIC_INLINE int min_func(int x, int y) { return x < y ? x : y; }

#define min(a,b) min_func((a),(b))

Then if you have a performance problem on some particular platform, and it turns out to be because that compiler fails to inline it, you can worry about whether on that compiler you need to define STATIC_INLINE differently (to static inline in C99, or static __inline for Microsoft, or whatever), or perhaps even implement the min macro differently. That level of optimisation ("does it get inlined?") isn't something you can do much about in portable code.

Answer 6

Personally I don't think this code is safe at all, even if we don't consider multithreaded scenarios.

Take the following evaluation:

int a = min(min(1, 2), min(3, 4));

How will this expand properly and evaluate?

1. Evaluate min(1, 2) and assign value to x_GLOB (outer macro):
   • x_GLOB = 1
   • y_GLOB = 2
   • evaluate min, result is 1, assign to x_GLOB (for outer macro)
2. Evaluate min(3, 4) and assign value to y_GLOB (outer macro):
   • x_GLOB = 3 (uh-oh, 1 from the first expansion is now clobbered)
   • y_GLOB = 4
   • evaluate min, result is 3, assign to y_GLOB (for outer macro)
3. Evaluate min(a, b) where a is min(1, 2) and b is min(3, 4)
   • evaluate min of x_GLOB (which is 3) and y_GLOB (which is 3)
   • result of total evaluation is 3

Or am I missing something here?