The Great -Wundef purge

As of glibc-2.21 (2015-02) there should be no more -Wundef warnings during the building of glibc (excluding tests). The exception are those warnings in tests being worked around globally by -Wno-error=undef (Makeconfig: gccwarn += -Werror -Wno-error=undef).

In glibc we use a lot of macro API interfaces. These are interfaces where included code does different things depending on the macro defitions made by a given machine architecture. We are not going to remove these macro API interfaces any time soon, but a recent ABI scare has prompted the entire community to make them typo-proof. You make the macros typo-proof by using -Wundef and #if. You never use #ifdef or defined since they admit that the macro is not defined. You accept that each and every macro must always have a value no matter the configuration.

To this end we have enabled -Wundef in glibc right now and you will get warnings like this: 

In file included from ../sysdeps/x86_64/multiarch/init-arch.c:22:0,
                 from ../nptl/sysdeps/unix/sysv/linux/x86/init-arch.c:1:
../sysdeps/x86_64/multiarch/init-arch.c: In function ‘__init_cpu_features’:
../sysdeps/x86_64/multiarch/init-arch.h:160:36: warning: "FEATURE_INDEX_1" is not defined [-Wundef]
 # define index_Fast_Unaligned_Load FEATURE_INDEX_1
                                    ^

The fix for this failure is to ensure that FEATURE_INDEX_1 is always defined. In this particular case there are instances where it is not defined and the default undefined macro of 0 happens to do the right thing. In other cases it won't do the right thing. As you can see switching to -Wundef catches this problem.

Constructs to avoid

While trying to make the macro APIs typo-proof you may be tempted to do one or some of the following constructs. They are all still typo-prone and should be avoided.

The Centralized defaults pattern: 

#ifndef FOO
#define FOO 0
#endif

This is typo-prone because the developer may write #define F0O 1 (typo) by accident and never know that the default is being used instead of the expected value. The use of -Wundef doesn't help here because FOO always has value, but it is now typo-prone. The fix is to follow one of the best practices below.

Best Practice

In order of preference the following is the best practice collected from the community.

Remove the "macro API"

Avoid macro APIs in the first place. If you can remove the macro API from the design then there can be no typos and the compiler helps you with errors and warnings.

Always define all "macro API" values

The next best thing you can do, particularly if only a few files are involved is to always define the macros you need to use.

The only 100% bullet-proof-and-safe way to use the macro API pattern is to always define all macros that are required for the API to be complete. Do not rely on any defaults.

Use typo-proof "macro API" defaults

If you have a lot of files that use the macro API it may be useful to define defaults for the macros.

The defaults should always be defined in a foo-defaults.h header like this: 

/* The default for FOO is 0.  */
#define DEFAULT_FOO 0

To use the default you would do: 

/* Use the default for FOO.  */
#define FOO DEFAULT_FOO

It is critical the user define FOO themselves to avoid typos. This pattern catches typos in DEFAULT_FOO and later in uses of FOO if either was wrong.

To override FOO the user of the API would do: 

#define FOO 1

The defaults for FOO can never be overriden, they are global defaults. For example the following code is typo-prone: 

/* Let us override the defaults!  */
#undef DEFAULT_FOO
#define DEFAULT_FOO 1

You may typo to DEFAULT_FO0 and the original default would apply instead of the new one. It's less likely you'll make the typo twice, but it could happen via cut-and-paste, so we suggest against it. If you need to override the default consider adding another default e.g. 

/* We provide several defaults.  One for slow systems another for fast.  */
#define DEFAULT_FOO_FAST 0
#define DEFAULT_FOO_SLOW 1

Intermediate macro for external defines

The compiler and headers shared with glibc that have canonical sources elsewhere may not be easy to change to follow best practice as recommended for the glibc project. That's OK. You can still do a very good job by using another intermediate macro that is safe.

For example say the compiler defines __mips16 when it is generating MIPS16 code.

Instead of the following: 

#ifdef __mips16
...
#endif

Which is typo-prone to typos e.g. ___mips16 or _mips16, you could create an intermediate macro to make all of the uses safe. For example in a mips-defaults.h header you define: 

/* In a central internal header...  */
#ifdef __mips16
#define __glibc_mips16 1
#else
#define __glibc_mips16 0
#endif

Then in all the uses you would write: 

/* In a use of the macro...  */
#if __glibc_mips16
... /* All the uses are typo safe now.  */
#endif

It doesn't protect you from a compiler error e.g. failure to define __mips16, but it does protect you from typos scattered throughout the code that previously used __mips16.

Next steps?

None: Wundef (last edited 2015-10-19 15:32:45 by CarlosODonell)