Is it a bad practice to use #ifdef in code?

Question

I have to use lot of #ifdef i386 and x86_64 for architecture specific code and some times #ifdef MAC or #ifdef WIN32... so on for platform specific code.

We have to keep the common code base and portable.

But we have to follow the guideline that use of #ifdef is strict no. I dont understand why?

As a extension to this question I would also like to understand when to use #ifdef ?

For example, dlopen() cannot open 32 bit binary while running from 64 bit process and vice versa. Thus its more architecture specific. Can we use #ifdef in such situation?

Answer 1

With #ifdef instead of writing portable code, you're still writing multiple pieces of platform-specific code. Unfortunately, in many (most?) cases, you quickly end up with a nearly impenetrable mixture of portable and platform-specific code.

You also frequently get #ifdef being used for purposes other than portability (defining what "version" of the code to produce, such as what level of self-diagnostics will be included). Unfortunately, the two often interact, and get intertwined. For example, somebody porting some code to MacOS decides that it needs better error reporting, which he adds -- but makes it specific to MacOS. Later, somebody else decides that the better error reporting would be awfully useful on Windows, so he enables that code by automatically #defineing MACOS if WIN32 is defined -- but then adds "just a couple more" #ifdef WIN32 to exclude some code that really is MacOS specific when Win32 is defined. Of course, we also add in the fact that MacOS is based on BSD Unix, so when MACOS is defined, it automatically defines BSD_44 as well -- but (again) turns around and excludes some BSD "stuff" when compiling for MacOS.

This quickly degenerates into code like the following example (taken from #ifdef Considered Harmful):

#ifdef SYSLOG
#ifdef BSD_42
openlog("nntpxfer", LOG_PID);
#else
openlog("nntpxfer", LOG_PID, SYSLOG);
#endif
#endif
#ifdef DBM
if (dbminit(HISTORY_FILE) < 0)
{
#ifdef SYSLOG
    syslog(LOG_ERR,"couldn’t open history file: %m");
#else
    perror("nntpxfer: couldn’t open history file");
#endif
    exit(1);
}
#endif
#ifdef NDBM
if ((db = dbm_open(HISTORY_FILE, O_RDONLY, 0)) == NULL)
{
#ifdef SYSLOG
    syslog(LOG_ERR,"couldn’t open history file: %m");
#else
    perror("nntpxfer: couldn’t open history file");
#endif
    exit(1);
}
#endif
if ((server = get_tcp_conn(argv[1],"nntp")) < 0)
{
#ifdef SYSLOG
    syslog(LOG_ERR,"could not open socket: %m");
#else
    perror("nntpxfer: could not open socket");
#endif
    exit(1);
}
if ((rd_fp = fdopen(server,"r")) == (FILE *) 0){
#ifdef SYSLOG
    syslog(LOG_ERR,"could not fdopen socket: %m");
#else
    perror("nntpxfer: could not fdopen socket");
#endif
    exit(1);
}
#ifdef SYSLOG
syslog(LOG_DEBUG,"connected to nntp server at %s", argv[1]);
#endif
#ifdef DEBUG
printf("connected to nntp server at %s\n", argv[1]);
#endif
/*
* ok, at this point we’re connected to the nntp daemon
* at the distant host.
*/

This is a fairly small example with only a few macros involved, yet reading the code is already painful. I've personally seen (and had to deal with) much worse in real code. Here the code is ugly and painful to read, but it's still fairly easy to figure out which code will be used under what circumstances. In many cases, you end up with much more complex structures.

To give a concrete example of how I'd prefer to see that written, I'd do something like this:

if (!open_history(HISTORY_FILE)) {
    logerr(LOG_ERR, "couldn't open history file");
    exit(1);
}

if ((server = get_nntp_connection(server)) == NULL) {
    logerr(LOG_ERR, "couldn't open socket");
    exit(1);
}

logerr(LOG_DEBUG, "connected to server %s", argv[1]);

In such a case, it's possible that our definition of logerr would be a macro instead of an actual function. It might be sufficiently trivial that it would make sense to have a header with something like:

#ifdef SYSLOG
    #define logerr(level, msg, ...) /* ... */
#else
    enum {LOG_DEBUG, LOG_ERR};
    #define logerr(level, msg, ...) /* ... */
#endif

[for the moment, assuming a preprocessor that can/will handle variadic macros]

Given your supervisor's attitude, even that may not be acceptable. If so, that's fine. Instead a macro, implement that capability in a function instead. Isolate each implementation of the function(s) in its own source file and build the files appropriate to the target. If you have a lot of platform-specific code, you usually want to isolate it into a directory of its own, quite possibly with its own makefile¹, and have a top-level makefile that just picks which other makefiles to invoke based on the specified target.

---

1. Some people prefer not to do this. I'm not really arguing one way or the other about how to structure makefiles, just noting that it's a possibility some people find/consider useful.

Answer 2

You should avoid #ifdef whenever possible. IIRC, it was Scott Meyers who wrote that with #ifdefs you do not get platform-independent code. Instead you get code that depends on multiple platforms. Also #define and #ifdef are not part of the language itself. #defines have no notion of scope, which can cause all sorts of problems. The best way is to keep the use of the preprocessor to a bare minimum, such as the include guards. Otherwise you are likely to end up with a tangled mess, which is very hard to understand, maintain, and debug.

Ideally, if you need to have platform-specific declarations, you should have separate platform-specific include directories, and handle them appropriately in your build environment.

If you have platform specific implementation of certain functions, you should also put them into separate .cpp files and again hash them out in the build configuration.

Another possibility is to use templates. You can represent your platforms with empty dummy structs, and use those as template parameters. Then you can use template specialization for platform-specific code. This way you would be relying on the compiler to generate platform-specific code from templates.

Of course, the only way for any of this to work, is to very cleanly factor out platform-specific code into separate functions or classes.