views:

633

answers:

7

I'm trying to streamline large chunk of legacy C code in which, even today, before doing the build guy who maintains it takes a source file(s) and manually modifies the following section before the compilation based on the various types of environment.

The example follows but here's the question. I'm rusty on my C but I do recall that using #ifdef is discouraged. Can you guys offer better alternative? Also - I think some of it (if not all of it) can be set as environment variable or passed in as a parameter and if so - what would be a good way of defining these and then accessing from the source code?

Here's snippet of the code I'm dealing with

#define DAN          NO
#define UNIX         NO
#define LINUX        YES
#define WINDOWS_ES   NO
#define WINDOWS_RB   NO

/* Later in the code */
#if ((DAN==1) || (UNIX==YES))
#include <sys/param.h>
#endif

#if ((WINDOWS_ES==YES) || (WINDOWS_RB==YES) || (WINDOWS_TIES==YES))
#include <param.h>
#include <io.h>
#include <ctype.h>
#endif

/* And totally insane harcoded paths */
#if (DAN==YES)
char MasterSkipFile[MAXSTR] = "/home/dp120728/tools/testarea/test/MasterSkipFile";
#endif

#if (UNIX==YES)
char MasterSkipFile[MAXSTR] = "/home/tregrp/tre1/tretools/MasterSkipFile";
#endif

#if (LINUX==YES)
char MasterSkipFile[MAXSTR] = "/ptehome/tregrp/tre1/tretools/MasterSkipFile";
#endif

/* So on for every platform and combination */
+12  A: 

Sure, you can pass -DWHATEVER on the command line. Or -DWHATEVER_ELSE=NO, etc. Maybe for the paths you could do something like

char MasterSkipFile[MAXSTR] = SOME_COMMAND_LINE_DEFINITION;

and then pass

-DSOME_COMMAND_LINE_DEFINITION="/home/whatever/directory/filename"

on the command line.

Carl Norum
I agree that the MasterSkipFile definition should be along the lines shown; what can be less satisfactory is burying the definitions in the build control files. It would be better to have a 'config.h' file (that's the name created by autoconf) or 'porting.h' or whatever that is under VCS and is scanned by programs such as cscope or an iDE. I work on a system where there are 60 defines that occur only in makefiles; it is very hard to work out which of the #ifdef's in the source code are actually defined still, because the defines aren't visible to cscope etc.
Jonathan Leffler
I will not be dealing with command line per say. It will be `make something` triggered by external build process (maven)
DroidIn.net
What @Jonathan is trying to say is that an autogenerated header might be a better solution for you. That is quite possible, and might clean up complicated makefile messes.
Carl Norum
My problem is - it's legacy code that goes away eventually so least I learn about it and least I modify it the better. I think I'll end up with combination of command line params, predefined platform-specific .h files and #ifdef (oh, well)
DroidIn.net
+3  A: 

Its much saner to use :

#if SOMETHING

.. from platform to platform, to avoid confusing broken preprocessors. However any modern compiler should effectively argue your case in the end. If you give more details on your platform, compiler and preprocessor you might receive a more concise answer.

Conditional compilation, given the plethora of operating systems and variants therein is a necessary evil. if, ifdef, etc are most decidedly not an abuse of the preprocessor, just exercising it as intended.

Tim Post
+4  A: 

One thing we used to do is have a generated .h file with these definitions, and generate it with a script. That helped us get rid of a lot of brittle #ifs and #ifdefs

You need to be careful about what you put there, but machine-specific parameters are good candidates - this is how autoconf/automake work.

EDIT: in your case, an example would be to use the generated .h file to define INCLUDE_SYS_PARAM and INCLUDE_PARAM, and in the code itself use:

#ifdef INCLUDE_SYS_PARAM
#include <sys/param.h>
#endif

#ifdef INCLUDE_PARAM
#include <param.h>
#endif

Makes it much easier to port to new platforms - the existence of a new platform doesn't trickle into the code, only to the generated .h file.

orip
+3  A: 

My preferred way would be to have the build system do the OS detection. Complex cases you'd want to isolate the machine-specific stuff into a single source file, and have completely different source files for the different OSes.

So in this case, you'd have a #include "OS_Specific.h" in that file. You put the different includes, and the definition of MasterSkipFile for this platform. You can select between them by specifying different -I (include path directories) on your compiler command line.

The nice thing about doing it this way is that somebody trying to figure out the code (perhaps debugging) doesn't have to wade through (and possibly be misled by) phantom code for a platform they aren't even running on.

T.E.D.
+2  A: 

I've seen build systems in which most of the source files started something off like this:

#include PLATFORM_CONFIG
#include BUILD_CONFIG

and the compiler was kicked off with:

cc -DPLATFORM_CONFIG="linuxconfig.h" -DBUILD_CONFIG="importonlyconfig.h"

(this may need backslash escapes)

this had the effect of letting you separate out the platform settings in one set of files and the configuration settings in another. Platform settings manages handling library calls that may not exist on one platform or not in the right format as well as defining important size dependent types--things that are platform specific. Build settings handles what features are being enabled in the output.

plinth
+4  A: 

Platform specific configuration headers

I'd have a system to generate the platform-specific configuration into a header that is used in all builds. The AutoConf name is 'config.h'; you can see 'platform.h' or 'porting.h' or 'port.h' or other variations on the theme. This file contains the information needed for the platform being built. You can generate the file by copying a version-controlled platform-specific variant to the standard name. You can use a link instead of copying. Or you can run configuration scripts to determine its contents based on what the script finds on the machine.

Default values for configuration parameters

The code:

#if (DAN==YES)
char MasterSkipFile[MAXSTR] = "/home/dp120728/tools/testarea/MasterSkipFile";
#endif

#if (UNIX==YES)
char MasterSkipFile[MAXSTR] = "/home/tregrp/tre1/tretools/MasterSkipFile";
#endif

#if (LINUX==YES)
char MasterSkipFile[MAXSTR] = "/ptehome/tregrp/tre1/tretools/MasterSkipFile";
#endif

Would be better replaced by:

#ifndef MASTER_SKIP_FILE_PATH
#define MASTER_SKIP_FILE_PATH "/opt/tretools/MasterSkipFile"
#endif

const char MasterSkipFile[] = MASTER_SKIP_FILE_PATH;

Those who want the build in a different location can set the location via:

-DMASTER_SKIP_FILE_PATH='"/ptehome/tregtp/tre1/tretools/PinkElephant"'

Note the use of single and double quotes; try to avoid doing this on the command line with backslashes in the path. You can use a similar default mechanism for all sorts of things:

#ifndef DEFAULTABLE_PARAMETER
#define DEFAULTABLE_PARAMETER default_value
#endif

If you choose your defaults well, this can save a lot of energy.

Relocatable software

I'm not sure about the design of the software that can only be installed in one location. In my book, you need to be able to have the old version 1.12 of the product installed on the machine at the same time as the new 2.1 version, and they should be able to operate independently. A hard-coded path name defeats that.

Parameterize by feature not platform

The key difference between the AutoConf tools and the average alternative system is that the configuration is done based on features, not on platforms. You parameterize your code to identify a feature that you want to use. This is crucial because features tend to appear on platforms other than the original. I look after code where there are lines like:

#if defined(SUN4) || defined(SOLARIS_2) || defined(HP_UX) || \
    defined(LINUX) || defined(PYRAMID) || defined(SEQUENT) || \
    defined(SEQUENT40) || defined(NCR) ...
#include <sys/types.h>
#endif

It would be much, much better to have:

#ifdef INCLUDE_SYS_TYPES_H
#include <sys/types.h>
#endif

And then on the platforms where it is needed, generate:

#define INCLUDE_SYS_TYPES_H

(Don't take this example header too literally; it is the concept I am trying to get over.)

Treat platform as a bundle of features

As a corollary to the previous point, you do need to detect platform and define the features that are applicable to that platform. This is where you have the platform-specific configuration header which defines the configuration features.

Product features should be enabled in a header

(Elaborating on a comment I made to another answer.)

Suppose you have a bunch of features in the product that need to be included or excluded conditionally. For example:

KVLOCKING
B1SECURITY
C2SECURITY
DYNAMICLOCKS

The relevant code is included when the appropriate define is set:

#ifdef KVLOCKING
...KVLOCKING stuff...
#else
...non-KVLOCKING stuff...
#endif

If you use a source code analysis tool like cscope, then it is helpful if it can show you when KVLOCKING is defined. If the only place where it is defined is in some random Makefiles scattered around the build system (let's assume there are a hundred sub-directories that are used in this), it is hard to tell whether the code is still in use on any of your platforms. If the defines are in a header somewhere - the platform specific header, or maybe a product release header (so version 1.x can have KVLOCKING and version 2.x can include C2SECURITY but 2.5 includes B1SECURITY, etc), then you can see that KVLOCKING code is still in use.

Believe me, after twenty years of development and staff turnover, people don't know whether features are still in use or not (because it is stable and never causes problems - possibly because it is never used). And if the only place to find whether KVLOCKING is still defined is in the Makefiles, then tools like cscope are less helpful - which makes modifying the code more error prone when trying to clean up later.

Jonathan Leffler
Thanks Jonathan!
DroidIn.net
+1  A: 

Generalities

I'm a heretic who has been cast out from the Church of the GNU Autotools. Why? Because I like to understand what the hell my tools are doing. And because I've had the experience of trying to combine two components, each of which insisted on a different, incompatible version of autotools being the default version installed on my computer.

I work by creating one .h file or .c filed for every combination of platform and significant abstraction. I work hard to define a central .h file that says what the interface is. Often this means I wind up creating a "compatibility layer" that insulates me from differences between platforms. Often I wind up using ANSI Standard C whenever possible, instead of platform-specific functionality.

I sometimes write scripts to generate platform-dependent files. But the scripts are always written by hand and documented, so I know what they do.

I admire Glenn Fowler's nmake and Phong Vo's iffe (if feature exists), which I think are better engineered than the GNU tools. But these tools are part of the AT&T Software Technology suite, and I haven't been able to figure out how to use them without buying into the whole AST way of doing things, which I don't always understand.

Your example

There clearly needs to be

extern char MasterSkipFile[];

in a .h file somewhere, and you can then link against a suitable .o.

The conditional inclusion of the "right set of .h files for the platform" is something I would handle by trying to stick to ANSI C when possible, and when not possible, defining a compatibility layer in a platform-specific .h file. As it is, I can't tell what names the #includes are trying to import, so I can't give more specific advice.

Norman Ramsey