Closed Bug 211501 Opened 18 years ago Closed 17 years ago

NSPR should provide getaddrinfo

Categories

(NSPR :: NSPR, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: darin.moz, Assigned: darin.moz)

References

Details

Attachments

(1 file, 7 obsolete files)

NSPR should provide a function like getaddrinfo (RFC 2553), which is now
available on many operating systems.  In particular this function is needed in
order to support IPv6 under WinXP.
Attached patch v1 patch (obsolete) — Splinter Review
This is based on an API that wtc and I came up with a couple weeks ago.
I have tested this patch under WinXP, Linux-RH9, and MacOSX 10.2.  I plan to
test it out on other versions of windows shortly.  Any help making sure it does
the right thing on other platforms would be appreciated.

Actually, I'm pretty sure we don't have |struct addrinfo| on all UNIX platforms,
but the patch assumes that we do.  That'll have to change.
Status: NEW → ASSIGNED
Attachment #126959 - Flags: review?(wtc)
Attached patch v1.1 patch (obsolete) — Splinter Review
slightly revised patch:

 o  PR_GetAddrInfoByName now includes |af| and |flags| parameters.
 o  wrapped calls to dlopen/dlsym/dlclose with #ifdef USE_DLFCN.
 o  set correct error code when getaddrinfo fails.
Attachment #126959 - Attachment is obsolete: true
Attachment #127501 - Flags: review?(wtc)
Attachment #126959 - Flags: review?(wtc)
Attachment #127501 - Attachment is obsolete: true
Attachment #127501 - Flags: review?(wtc)
Attached patch v1.2 patch (obsolete) — Splinter Review
this patch uses PR_FindFunctionSymbolAndLibrary instead of platform specific
methods.  it also fixes some other potential compilation problems on other
platforms.  i've compiled this under WinXP (MSVC6), RH9-Linux (GLIBC 2.3),
MacOSX 10.2.
Attached patch v1.3 patch (obsolete) — Splinter Review
left out one thing from last patch.
Attachment #127557 - Attachment is obsolete: true
Attachment #127558 - Flags: review?(wtc)
cc'ing some folks involved with mozilla ports.  if you guys could compile the
v1.3 patch for me, and let me know if it breaks and/or let me know if you see
anything in it that needs to be modified for your platform, it'd be greatly
appreciated.  also, if you know of any other ports maintainers that should be
cc'd, please add them.  thanks!
For OpenVMS I need the ability to specify at run time the name of the symbol
that we try to dynamically look up, in the same way that we already do for
GETIPNODEBYXXXX and FREEHOSTENT. See
http://lxr.mozilla.org/nspr/source/nsprpub/pr/src/io/pripv6.c#295 for details.
colin: so, OpenVMS does indeed provide getaddrinfo then?
> colin: so, OpenVMS does indeed provide getaddrinfo then?

In more recent versions of TCP/IP Services for OpenVMS, which have support for
IPv6, yes. But not everyone who is running TCP/IP Services for OpenVMS (UCX) is
running a recent version.

And UCX is only one of three commonly used IP stacks on OpenVMS. I don't think
the other two are as far along in their IPv6 support.
ok, thanks colin... i'll modify the patch as you suggested.
Attached patch v1.4 patch (obsolete) — Splinter Review
revised patch:

1- added getenv calls for VMS
2- added AI_ADDRCONFIG per wtc (following apache's usage of getaddrinfo)
Attachment #127558 - Attachment is obsolete: true
Attachment #127558 - Flags: review?(wtc)
Comment on attachment 127743 [details] [diff] [review]
v1.4 patch

wtc: nevermind the "RFC 2553" mentioned in the .h file... i've changed that to
RFC 3493 in my source tree :)
Attachment #127743 - Flags: review?(wtc)
For BeOS:  I will verify this the week after next.  Sorry I can't do it sooner (was in thehostpital, now going on vacation)  I will let you know when I get back
Builds on OS/2.  Other than trying to build this with the nsDnsService rewrite,
is there a way to test this new function?
Javier: no, there isn't any NSPR test for these functions (yet).  if you want to
easily apply the full DNS rewrite patch, you can do so by downloading the entire
patch (including the changes to NSPR) from here:

  http://friedfish.homeip.net/builds/dns/dns-20030718.diff

NOTE: this is a dyndns hostname, so you might have trouble loading this page if
your firewall blocks internal access to dyndns hosts (as is the case here at AOL).
Darin:  Thanks for the patch. I found that I wasn't exporting the new functions
out of NSPR. I need to add this to nsprpub/pr/src/nspr.def:

;+NSPR_4.5 {
;+	global:
		PR_GetAddrInfoByName;
		PR_FreeAddrInfo;
		PR_EnumerateAddrInfo;
		PR_GetCanonNameFromAddrInfo;
;+} NSPR_4.4;

However, I just made those NSPR numbers up.  WTC, is the correct thing to do? 
Should I just put everything in the NSPR_4.4 block?
These new functions should be in the NSPR_4.5 block.
(NSPR 4.4, the version of NSPR in Mozilla 1.4, has
been released.)
Attached patch OpenVMS changes needed (obsolete) — Splinter Review
Wan-Teh, we also need the same nspr.def changes in the OpenVMS equivalent.
Patch is attached. I also noticed that PR_GetPathSeparator snuck into nspr.def
without being added to nspr_symvec.opt, so I've added that too.
Attached patch nspr.def changes (obsolete) — Splinter Review
Patch for the nspr.def changes.  With this, it builds and works on OS/2 (tested
with full nsDnsService rewrite patch).
Attached patch v1.5 patchSplinter Review
revised to load getaddrinfo/freeaddrinfo properly on WinXP.  this patch also
includes the last two attachments for OpenVMS and OS/2.
Attachment #127743 - Attachment is obsolete: true
Attachment #128152 - Attachment is obsolete: true
Attachment #128160 - Attachment is obsolete: true
Attachment #127743 - Flags: review?(wtc)
Comment on attachment 128183 [details] [diff] [review]
v1.5 patch

wtc: this patch does not include making PR_GetAddrInfoByName return PRAddrInfo*
  ... in case you have other comments, i'd prefer to fix all together.
Attachment #128183 - Flags: review?(wtc)
Darin, I made the first pass through your patch.  I did some
editing and checked in your patch on the NSPR_GETADDRINFO_BRANCH
of mozilla/nsprpub.

I have not made any API changes, so your code should compile and
link just fine against NSPR_GETADDRINFO_BRANCH.  I did make a
semantic change to PR_EnumerateAddrInfo: PR_EnumerateAddrInfo
now returns NULL when there is no more address to enumerate, so
you should not use the address when PR_EnumerateAddrInfo returns
NULL.  This allows you to write the enumeration using a while
loop like this:

    void *iter;
    PRAddrInfo *ai;
    PRNetAddr addr;
    PRUint16 port;

    iter = NULL;
    while ((iter = PR_EnumerateAddrInfo(iter, ai, port, &addr)) != NULL) {
        /* use 'addr' */
    }

You'd need to modify your patches in bug 205726 accordingly.  The
reason for this change is to be consistent with PR_EnumerateHostEnt.
Let me know if you like this change.
I also changed the meaning of the HAVE_GETADDRINFO
macro (it's also renamed _PR_HAVE_GETADDRINFO).
It used to mean "the target OS has get getaddrinfo";
it now means "the current or a later version of the
target OS has getaddrinfo".  Again, this is to be
consistent with the meaning of _PR_HAVE_GETIPNODEBYNAME.
wtc: i took a look at the changes on the branch, and they look great.  i really
like the way you cleaned up the #defines :)


>I have not made any API changes, so your code should compile and
>link just fine against NSPR_GETADDRINFO_BRANCH.  

but do you still want to make PR_GetAddrInfoByName return |PRAddrInfo*|?


>I did make a semantic change to PR_EnumerateAddrInfo: PR_EnumerateAddrInfo
>now returns NULL when there is no more address to enumerate, so
>you should not use the address when PR_EnumerateAddrInfo returns NULL.

this is what i have been implementing or so i thought.  was there a case where
this was somehow not true (in the v1.5 patch)?


also, let me know what you think remains to be done with this patch.  thanks!
Is this getting checked in for 1.5?
colin: no, i've decided to postpone the DNS rewrite until the trunk opens for
1.6 alpha.  reviews on the necko portion are still pending :(
ok, this is getting checked in shortly.  wtc gave me approval yesterday to land
this on the NSPRPUB_PRE_4_2_CLIENT_BRANCH and on the NSPR trunk.
ok, this is now checked in on the NSPR trunk and NSPRPUB_PRE_4_2_CLIENT_BRANCH.
wtc says that there are likely build problems on OSF1 that we will need to fix.

if anyone happens to have access to an OSF1 box, please let us know!
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Blocks: 175340
Whiteboard: [4.5]
Filed bug 219104 on a Mac OS X issue in this patch.
I've yet to verify it, but this checkin might have caused a problem in Windows
98.  Entering an ip address instead of dns name causes a "could not be found" error.
The problem you described in comment 30 is introduced by
this checkin and is the subject of bug 219376.
Attachment #128183 - Flags: review?(wchang0222)
Whiteboard: [4.5]
Target Milestone: --- → 4.5
You need to log in before you can comment on or make changes to this bug.