Problematic port field in struct ip_service in Samba.

[Jeremy Allison]

Hi Team & all interested parties,

Inside Samba we use a struct:

source3/include/smb.h:

struct ip_service {
        struct sockaddr_storage ss;
        unsigned port;
};

It's used in these files:

source3/include/proto.h
source3/include/smb.h
source3/libads/kerberos.c
source3/libads/ldap.c
source3/libsmb/dsgetdcname.c
source3/libsmb/namecache.c
source3/libsmb/namequery.c
source3/libsmb/namequery.h
source3/libsmb/namequery_dc.c
source3/torture/torture.c
source3/utils/net_lookup.c
source3/winbindd/winbindd_cm.c

Looking carefully I haven't been able
to find any place where we actually
use the 'port' field of this struct
other than (a) setting it to zero
or (b) setting it from the return
from a SRV record lookup when that
lookup returns a port entry.

As far as I can see it's never actually
used to select an outgoing port for
any connections :-).

In fact in many of the name resolv
functions internally (resolve_hosts()
for example which uses getaddrinfo() directly)
we explicitly set it to zero.

I'm currently working on fixing
up some of our internal name resolution
functions to use a pthreadpool async
version of glibc getaddrinfo() which
is already thread safe, to improve
our name resolution in poor DNS
environments.

It would make some of this code
much easier if I could just ditch
the 'port' requirement and set it
to zero. It's only actually correctly
returned from DNS lookups when we're
using our own SRV record lookup code
anyway, so I'm pretty sure it can't
be relied upon.

I have a working version that preserves
the existing 'return port' semantics,
but it makes the code uglier than I'd
like and prevents me from easily
using it to parallelize many of our
DNS lookups (like the ones done from
winbindd for example).

Eventually I'd like to replace all
uses of struct ip_service -> struct sockaddr_storage,
but that's a bigger cleanup for another
day.. :-).

Any objections to silently setting
port = 0 for name resolution ? It's
done in many of the existing code
paths anyway so we can't rely on port
being set correctly in returns from
internal_resolve_name(), plus we
never actually use it :-).

Thoughts ?

Jeremy.