apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Lucian Adrian Grijincu" <lucian.griji...@gmail.com>
Subject Re: [Vote] Release APR 1.2.9/0.9.14 and apr-iconv 1.2.0
Date Wed, 06 Jun 2007 22:08:05 GMT
On 6/7/07, William A. Rowe, Jr. <wrowe@rowe-clan.net> wrote:
> Lucian Adrian Grijincu wrote:
> >
> > I hear that there are real performance reasons to maintain
> > AI_ADDRCONFIG for AF_UNSPEC:
> > http://www.ops.ietf.org/lists/v6ops/v6ops.2003/msg01377.html
> >
> > I first though we could do a strcmp to check for "127.0.0.1" or "::1",
> > but it's not going to work, because users may have altered the hosts
> > file to set a specific hostname to loopback and this code should be
> > able to get things like "localhost" as a valid parameter.
>
> Correct, that won't work.  Although looking for "127." or "::" prefixes
> might be a legimate test, however "localhost" won't resolve.
>
> Remember that setting up the loopback IP's .1, .2, .3, .4 all on the
> same port might be a reasonable design of a local app and wouldn't pass
> if you looked for the full loopback IP.
>
> > I see three getaways:
> > 1. kill AI_ADDRCONFIG for APR_UNSPEC
> > 2. document "::1" and any other link-local addresses and hostnames as
> > invalid if APR_UNSPEC is used.
> > 3. retry without AI_ADDRCONFIG if first try fails
> >
> > 1. BAD: this is the only place where AI_ADDRCONFIG is used and not
> > using it would mean a performance regression for applications that do
> > not care about loopback.
> > 2. UGLY: APR should work arround platform specific issues and present
> > a simple interface. Having the user check for link-localness
> > beforehand is ... ugly. :)
> > 3. GOOD: We already retry for some other errors and retrying would
> > only harm performance-wise only in case of a:
> >  a) link-local address or hostname. This should be resolved locally,
> > which means the drawback is little
> >  b) failing external lookup. This can mean we'll take longer to say:
> > "I can't find the hostname you specified", or (not very probable)
> > manage to find it at the second try
> >
> >
> > I've attached a patch against the tarball that does what's needed for 3.
> >
> > Rebuilt from scratch + patch:
> >   finally, all test pass now :)
>
> I'm quite partial to your third solution, I trust from performance that
> this is the greatest net efficiency (but per platform tests would be needed
> to confirm this).  This is worth dumping 1.2.9 and rerolling 1.2.10 IMHO
> (along with other small fixes that are identified today or tomorrow morning.)
>
>

Hi Bill,

Please look at the immensity of the patch I submitted:

339c339
<     if ((error == EAI_BADFLAGS || error == EAI_ADDRFAMILY ) &&
family == APR_UNSPEC) {
---
>     if (error == EAI_BADFLAGS && family == APR_UNSPEC) {


which comes in this context:
    error = getaddrinfo(hostname, servname, &hints, &ai_list);
#ifdef HAVE_GAI_ADDRCONFIG
    if ((error == EAI_BADFLAGS || error == EAI_ADDRFAMILY ) && family
== APR_UNSPEC) {
        /* Retry with no flags if AI_ADDRCONFIG was rejected. */
        hints.ai_flags = 0;
        error = getaddrinfo(hostname, servname, &hints, &ai_list);
    }
#endif

I agree that this thing needs to be rechecked on all platforms, it's
just the sane thing to do, but I'd suggest that the 1.2.10 contain
this check and not dump the AI_ADDRCONFIG flag altogether.


Disclaimer: I haven't worked with autoconf automake etc. and I really
don't have the time :(

can someone please illuminate me what needs to be done to have a
single source file be compiled and linked on all platforms?

I'd like to see this:
http://mail-archives.apache.org/mod_mbox/apr-dev/200705.mbox/%3c4d45da050705161718l54f7b079h83ade8a3cc19751c@mail.gmail.com%3e
included in APR. It's been discussed in december 2006 too:
http://mail-archives.apache.org/mod_mbox/apr-dev/200612.mbox/browser

It's an implementation of "rm -d -r" based only on APR objects.

--
Thank you,
Lucian Adrian Grijincu

Mime
View raw message