Re: [Patch] Fix ONE_DIRECTION undef warnings.

[Roland McGrath <roland at hack dot frob dot com>]

> That sounds like you're against the default header design. Could you clarify
> your position on that?

I am against anything that is prone to typos introducing silent errors when
we can come up with an alternative solution that is not a worse maintenance
burden in other regards.  That is an abstract position and that's all I
have because I don't have a specific alternative proposal for
iconv/skeleton.c at the moment.  I'm hoping to encourage other folks like
you to explore new alternatives that might satisfy my constraints better
than what we've discussed so far.

> (c) Uses that need to override defaults do so like this.
> 
> #undef FOO
> #define FOO 1

This remains typo-prone in the sense that if FOO is the name in the macro
API but an overriding use is instead written as:

	#undef	FO0
	#define FO0 1

then the default value of FOO reigns and this unused FO0 macro is never
diagnosed.

With this fundamental style of "macro API" there is an unavoidable
trade-off between avoiding this possibility and avoiding duplicative
default definitions in many places.  (The actual risk of error in the
duplicative situation can be avoided by not duplicating the default value
but instead duplicating "#define FOO FOO_DEFAULT" or suchlike.)  Neither
situation is particularly appealing.

In general, we avoid these problems best by avoiding the "macro API"
pattern whenever we can do something else that is cleaner without
sacrificing other important concerns (like performance).  Sometimes that's
hard, as is probably the case for iconv/skeleton.c.  An example of avoiding
it entirely would be using C variables instead, where:
	static const sometype name_with_typo = 23;
would generate an unused-variable varible warning if name_with_typo is not
the name actually used.  In the absence of other horrors like C++ with
template tricks, there probably isn't a way to both use this style and have
defaults not duplicated in each user.  With recent compilers, this style is
OK for performance concerns because the compiler optimizes away the
variable and constant-folds conditionals and calculations that use it.

Another example of how the typo-prone pattern can be avoided while sticking
to a "macro API" is to replace the "user defines a bunch of constants" API
with something where the user does:

	#define THING_WITH_PARAMS THING (2, 3, 4, 5)

i.e., to replace:

	#define THING_PARAM1	2
	#define THING_PARAM2	3
	#define THING_PARAM3	4
	#define THING_PARAM4	5

To avoid the duplicated-defaults issue that can be:

	#define THING_WITH_PARAMS THING (DEFAULT_PARAM1, 3, 4, 5)

But of course that has other maintainability issues fairly similar to the
current style requiring each user to define a macro for each parameter
(pervasive mechanical changes when a parameter is added, removed, or
renamed) and ones of its own (positional parameters are always hard to
interpret by eyeball).

So, like I said, I have not come up with a particular alternative that is a
great fit for the iconv/skeleton.c case.  But I hope these examples suggest
the kind of general rethinking that I hope we'll do when trying to make any
particular case along these lines cleaner and easier to maintain.


Thanks,
Roland