211501 - NSPR should provide getaddrinfo

Status: RESOLVED FIXED

(NSPR :: NSPR, defect)

Description

[Darin Fisher]

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.

Comment 1

[Darin Fisher]

Attachment: v1 patch

This is based on an API that wtc and I came up with a couple weeks ago.

Comment 2

[Darin Fisher]

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.

Comment 3

[Darin Fisher]

Attachment: v1.1 patch

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.

Comment 4

[Darin Fisher]

Attachment: v1.2 patch

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.

Comment 5

[Darin Fisher]

Attachment: v1.3 patch

left out one thing from last patch.

Comment 6

[Darin Fisher]

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!

Comment 7

[Colin Blake]

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.

Comment 8

[Darin Fisher]

colin: so, OpenVMS does indeed provide getaddrinfo then?

Comment 9

[Colin Blake]

> 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.

Comment 10

[Darin Fisher]

ok, thanks colin... i'll modify the patch as you suggested.

Comment 11

[Darin Fisher]

Attachment: v1.4 patch

revised patch:

1- added getenv calls for VMS
2- added AI_ADDRCONFIG per wtc (following apache's usage of getaddrinfo)

Comment 12

[Darin Fisher]

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 :)

Comment 13

[Paul]

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

Comment 14

[jhp (no longer active)]

Builds on OS/2.  Other than trying to build this with the nsDnsService rewrite,
is there a way to test this new function?

Comment 15

[Darin Fisher]

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).

Comment 16

[jhp (no longer active)]

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?

Comment 17

[Wan-Teh Chang]

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.)

Comment 18

[Colin Blake]

Attachment: OpenVMS changes needed

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.

Comment 19

[jhp (no longer active)]

Attachment: nspr.def changes

Patch for the nspr.def changes.  With this, it builds and works on OS/2 (tested
with full nsDnsService rewrite patch).

Comment 20

[Darin Fisher]

Attachment: v1.5 patch

revised to load getaddrinfo/freeaddrinfo properly on WinXP.  this patch also
includes the last two attachments for OpenVMS and OS/2.

Comment 21

[Darin Fisher]

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.

Comment 22

[Wan-Teh Chang]

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.

Comment 23

[Wan-Teh Chang]

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.

Comment 24

[Darin Fisher]

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!

Comment 25

[Colin Blake]

Is this getting checked in for 1.5?

Comment 26

[Darin Fisher]

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 :(

Comment 27

[Darin Fisher]

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.

Comment 28

[Darin Fisher]

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!

Comment 29

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Filed bug 219104 on a Mac OS X issue in this patch.

Comment 30

[Ere Maijala (slow)]

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.

Comment 31

[Wan-Teh Chang]

The problem you described in comment 30 is introduced by
this checkin and is the subject of bug 219376.