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: "Carlos O'Donell" <carlos at redhat dot com>
- To: Roland McGrath <roland at hack dot frob dot com>
- Cc: Steve Ellcey <sellcey at mips dot com>, libc-alpha at sourceware dot org
- Date: Thu, 01 May 2014 19:48:51 -0400
- 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> <20140501180031 dot CEE6D2C3A10 at topped-with-meat dot com>
On 05/01/2014 02:00 PM, Roland McGrath wrote:
>> 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.
I did mean "typo-proof."
>> 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.
I agree.
OK, first draft done, and sent to the list.
Cheers,
Carlos.