This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [Patch] Fix ONE_DIRECTION undef warnings.
- From: Roland McGrath <roland at hack dot frob dot com>
- To: "Carlos O'Donell" <carlos at redhat dot com>
- Cc: Steve Ellcey <sellcey at mips dot com>, libc-alpha at sourceware dot org
- Date: Thu, 1 May 2014 11:00:31 -0700 (PDT)
- Subject: Re: [Patch] Fix ONE_DIRECTION undef warnings.
- Authentication-results: sourceware.org; auth=none
- References: <c087928d-c0a6-4121-8236-84a1a9e59870 at BAMAIL02 dot ba dot imgtec dot org> <20140428174126 dot 18CE02C3A00 at topped-with-meat dot com> <535FFE81 dot 4060104 at redhat dot com> <1398802315 dot 14541 dot 48 dot camel at ubuntu-sellcey> <5360162B dot 90303 at redhat dot com> <1398877703 dot 5201 dot 10 dot camel at ubuntu-sellcey> <5361361F dot 3090304 at redhat dot com> <20140430175647 dot DC6392C39DE at topped-with-meat dot com> <53613A9F dot 1060701 at redhat dot com> <20140430181003 dot 8C5582C39D5 at topped-with-meat dot com> <53613EE5 dot 40300 at redhat dot com> <20140430200609 dot 999EB2C39F4 at topped-with-meat dot com> <53618490 dot 30307 at redhat dot com>
> I prefer avoiding ABI mistakes to avoiding duplication. My opinion is
> that the duplication is simply the way the "macro API" works, you define
> the macros for your implementation, and you define all of them.
That's an entirely reasonable position.
It highlights how poor an API it is.
> On top of that maintainers will likely just do:
[...]
> Precisely because positional parameters are harder to read.
>
> Then they will typo the params into:
[...]
Probably true. It wasn't a particularly satisfying pattern to begin with.
> You've only underscored the fact that the solution we should take as a first
> round is to define ONE_DIRECTION is all the sources files that include
> iconv/skeleton.c. It's the least error prone and is a full definition of the
> "macro API" in each use.
I don't object to that.
> The only alternative I can think of is a two step process as you suggest.
>
> Default header does:
>
> #define DEFAULT_FOO 0
>
> Override does:
>
> #define FOO DEFAULT_FOO
>
> Use does:
>
> #if FOO
> ...
> #endiff
>
> Which makes it typo-prone because the user *must* define
> the final value of the name.
I don't see how that is actually typo-prone at all. A typo anywhere in
the #define line results in a -Wundef error (aside from writing
DEFAULT_BAR when that exists but DEFAULT_FOO is what you meant, which
does not seem so likely).
Your following text makes me think you actually meant to write
"typo-proof" there. That was pretty meta, dude.
> If it does I'll put it on a wiki page and document best practice
> to help all the developers trying to fix -Wundef warnings.
This works, but it seems rather cumbersome. I'd say "best practice" is
to avoid macro APIs altogether. When doing so would violate some other
best practice such as avoiding code duplication, then one should craft a
scheme that optimizes for the characteristics we like as best one can in
the particular case. For cases that have fewer parameters than the
iconv/skeleton.c case, some more concise scheme might have all the
desireable characteristics. It seems most important to document those
characteristics we like as a check-list of things people should be
trying to hit with whatever details make most sense for the given case.
Thanks,
Roland