You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by David Reid <dr...@jetnet.co.uk> on 2000/11/16 12:56:06 UTC
OK...
OK so Jeff has convinced me about not needing apr_in_addr_t's. So I'm
going to work on removing the apr_in_addr_t from the code all together...
There are a lot of questions that start with ??? so answers from anyone with
a point of view please!
1. Modify connect to take an apr_sockaddr_t instead of a hostname... This
is a simple change so I'll make it sooner rather than later.
??? Do we change apr_gethostbyname to return an apr_sockaddr_t?
This would then give
apr_sockaddr_t *sa;
apr_gethostbyname(&sa, "hostname", pool);
apr_connect(sock, sa);
We don't seem to be using apr_gethostbyname so we should be safe in making
this change.
2. Are we agreed on Jeff's suggestions of
Add apr_pool_t * to apr_sockaddr_t.
apr_status_t apr_get_address(char **hostname, apr_interface_e which,
apr_socket_t *sock);
apr_status_t apr_get_nas(char **addr, apr_sockaddr_t *sa);
These are new additions sos houldn't interfere with any existing code.
3. Before we can take this further, I guess we need to add the following
though...
apr_status_t apr_compare_sockaddr(apr_sockaddr_t *a, apr_sockaddr_t *b);
Basically compare 2 apr_sockaddr_t... return APR_SUCCESS ot the compare
results, (1/-1 etc) this is needed in http_vhost...
?? Do we want to add a flag for what we're comparing? I mean do we have
APR_COMPARE_ADDRESS_ONLY
APR_COMPARE_PORT_ONLY
APR_COMPARE_ALL
??
apr_status_t apr_mask_sockaddr(apr_sockaddr_t *a, mask???);
This is based on what seems to be needed in mod_access. We need some way of
checking if an IP is valid for a given mask. I'm not sure how we'd do this
for v6, or even how we'd define the mask in a v6 environment, but it should
be possible.
??? What form should the mask take?
Comments??
david
Re: OK...
Posted by Jeff Trawick <tr...@bellsouth.net>.
"David Reid" <dr...@jetnet.co.uk> writes:
> Just a thought, should we define the socket families as APR values? This
> would mean that on a non-v6 platform we could still call create socket with
> APR_V6 as a family and we'd just return an error. Don't know if it makes
> sense or not...
sure... at config time we can figure out the right values and
stick them in apr.h I think; on a system with no AF_INET6 we wouldn't
want to accidently set APR_V6 to the value for an address family it
actually has :) I'm punting on this for now
> Also, do we want to be able to pass in a servicename for the port?
> Otherwise I think we should add a function to get the numeric port for a
> service name...
On the one hand, it would be nice to pass it in to apr_getaddrinfo()
since on platforms with the real getaddrinfo() we can pass it on
through and magic happens. Unfortunately, I think apr_getaddrinfo()
is going to get darn ugly what with the series of functions we try at
config time and to a lesser extent at run-time (for
!GETHOSTBYNAME_HANDLES_NAS) just to resolve the hostname/numeric
address string.
A separate function is probably simplifying overall.
> > There is no flags parameter for now, but we may need to allow that
> > eventually.
>
> If we think we'll need it should we add it now so we don't break the API
> when we do?
>
will-do (but Greg S. doesn't like that style so if he reads this then
watch out :) )
> > > > In addition to the changes you mentioned, I see apr_create_socket() as
> > > > extremely important in the short run and I think we should think about
> > > > apr_bind() working like apr_connect() (in other words, taking an
> > > > apr_sockaddr_t). That makes sense when the user has told us the local
> > > > interface address and we have to resolve it anyway. We have to keep
> > > > it from being painful when we just have the port number.
> > >
> > > OK, so again care to suggest the API definitions?
> >
> > I think apr_connect() is good the way it is.
> >
> > For apr_bind(), I see it looking like apr_connect():
> >
> > apr_status_t apr_bind(apr_socket_t *, apr_sockaddr_t *);
> >
> > Why?
>
> This makes sense.
>
> +1
I'll hold off on that one for now (until after the alpha maybe?). I
need to spend some time making an honest attempt at not breaking Win32
and OS/2 when I ship this patch.
--
Jeff Trawick | trawick@ibm.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: OK...
Posted by David Reid <dr...@jetnet.co.uk>.
> > > > This would then give
> > > > apr_sockaddr_t *sa;
> > > > apr_gethostbyname(&sa, "hostname", pool);
> > > > apr_connect(sock, sa);
> > >
> > > We need to add more arguments: address family for sure and possibly
> > > flags. Look at getaddrinfo(). And do we want to add an optional
> > > service name/port number parameter? That would build the complete
> > > sockaddr for us.
> >
> > OK. Care to suggest the definition?
>
> apr_status_t apr_getaddrinfo(apr_sockaddr_t *sa,
> const char *hostname,
> apr_int32_t family,
> apr_port_t port,
> apr_pool_t *p);
>
> hostname is one of three things:
> a) ptr to DNS name
> b) ptr to numeric address string
> c) NULL if we don't have a name/address and just want to build a
> sockaddr with address INADDR[6]_ANY and some port.
+1 for this and your subsequent patch.
Just a thought, should we define the socket families as APR values? This
would mean that on a non-v6 platform we could still call create socket with
APR_V6 as a family and we'd just return an error. Don't know if it makes
sense or not...
Also, do we want to be able to pass in a servicename for the port?
Otherwise I think we should add a function to get the numeric port for a
service name...
>
> family is AF_UNSPEC if we don't care, AF_INET or AF_INET6 otherwise.
> You can't have hostname == NULL and family == AF_UNSPEC.
>
> port will be stored into the new sockaddr; pass zero if you don't
> care.
>
> There is no flags parameter for now, but we may need to allow that
> eventually.
If we think we'll need it should we add it now so we don't break the API
when we do?
>
> > > In addition to the changes you mentioned, I see apr_create_socket() as
> > > extremely important in the short run and I think we should think about
> > > apr_bind() working like apr_connect() (in other words, taking an
> > > apr_sockaddr_t). That makes sense when the user has told us the local
> > > interface address and we have to resolve it anyway. We have to keep
> > > it from being painful when we just have the port number.
> >
> > OK, so again care to suggest the API definitions?
>
> I think apr_connect() is good the way it is.
>
> For apr_bind(), I see it looking like apr_connect():
>
> apr_status_t apr_bind(apr_socket_t *, apr_sockaddr_t *);
>
> Why?
This makes sense.
+1
>
> This allows an app to use apr_getaddrinfo(), which will allocate a
> socket address, store the port in the right place, and perhaps
> handle a local interface address specified by the user.
david
Re: OK...
Posted by Jeff Trawick <tr...@bellsouth.net>.
"David Reid" <dr...@jetnet.co.uk> writes:
> > > This would then give
> > > apr_sockaddr_t *sa;
> > > apr_gethostbyname(&sa, "hostname", pool);
> > > apr_connect(sock, sa);
> >
> > We need to add more arguments: address family for sure and possibly
> > flags. Look at getaddrinfo(). And do we want to add an optional
> > service name/port number parameter? That would build the complete
> > sockaddr for us.
>
> OK. Care to suggest the definition?
apr_status_t apr_getaddrinfo(apr_sockaddr_t *sa,
const char *hostname,
apr_int32_t family,
apr_port_t port,
apr_pool_t *p);
hostname is one of three things:
a) ptr to DNS name
b) ptr to numeric address string
c) NULL if we don't have a name/address and just want to build a
sockaddr with address INADDR[6]_ANY and some port.
family is AF_UNSPEC if we don't care, AF_INET or AF_INET6 otherwise.
You can't have hostname == NULL and family == AF_UNSPEC.
port will be stored into the new sockaddr; pass zero if you don't
care.
There is no flags parameter for now, but we may need to allow that
eventually.
> > In addition to the changes you mentioned, I see apr_create_socket() as
> > extremely important in the short run and I think we should think about
> > apr_bind() working like apr_connect() (in other words, taking an
> > apr_sockaddr_t). That makes sense when the user has told us the local
> > interface address and we have to resolve it anyway. We have to keep
> > it from being painful when we just have the port number.
>
> OK, so again care to suggest the API definitions?
I think apr_connect() is good the way it is.
For apr_bind(), I see it looking like apr_connect():
apr_status_t apr_bind(apr_socket_t *, apr_sockaddr_t *);
Why?
This allows an app to use apr_getaddrinfo(), which will allocate a
socket address, store the port in the right place, and perhaps
handle a local interface address specified by the user.
--
Jeff Trawick | trawick@ibm.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: OK...
Posted by Jeff Trawick <tr...@bellsouth.net>.
"David Reid" <dr...@jetnet.co.uk> writes:
> [OK, I'm getting better at this Reply To All stuff...]
I still haven't gotten the hang of it :( Maybe I need to study the
GNUS manual to figure out how to avoid editing the cc line to keep me
out of it.
> > > 1. Modify connect to take an apr_sockaddr_t instead of a hostname...
> This
> > > is a simple change so I'll make it sooner rather than later.
> >
> > yes...
>
> OK, I've done some work on this but am getting an error now
> Could not connect: Can't assign requested address (49)
>
> I've got to drive to Suffolk in a short while so I'll post the patch at the
> end of this message and let someone else figure out where I've screwed up!
> :)
I'll start working on that.
> > > 2. Are we agreed on Jeff's suggestions of
> > > Add apr_pool_t * to apr_sockaddr_t.
> > > apr_status_t apr_get_address(char **hostname, apr_interface_e which,
> > > apr_socket_t *sock);
> > > apr_status_t apr_get_nas(char **addr, apr_sockaddr_t *sa);
> > > These are new additions sos houldn't interfere with any existing
> > > code.
> >
> > I am.
>
> Good.
There was an implied ":)" after "I am."
more comments on the rest later... for now I'll look at apr_connect()
and make an attempt at fixing the Win32 and OS/2 builds to include
apr_inet_ntop() and apr_inet_pton().
--
Jeff Trawick | trawick@ibm.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: OK...
Posted by Jeff Trawick <tr...@bellsouth.net>.
"David Reid" <dr...@jetnet.co.uk> writes:
> OK, I've done some work on this but am getting an error now
> Could not connect: Can't assign requested address (49)
I have it working on Linux now...
Changes to your patch or to other code to get it to work:
1) The existing apr_set_port() doesn't get along with the new
apr_connect(). When apr_connect() replaces the remote sockaddr,
we lose the port set previously.
For now, I added apr_set_sockaddr_port(), but I think we just
need to change apr_set_port() to take a sockaddr instead of a
socket and a local/remote flag.
2) socket address-related vars weren't set up in apr_gethostbyname()
when we created the sockaddr; I hit a segfault due to this
3) I made a fix/tweak to get_local_addr() (not shown below; will be
committed shortly); I don't *think* it was required but I may be
confused
New patch:
(Setting the port in client.c is in a different order than in your
patch due to my initial confusion about why the remote port was
getting lost.)
Index: include/apr_network_io.h
===================================================================
RCS file: /home/cvspublic/apache-2.0/src/lib/apr/include/apr_network_io.h,v
retrieving revision 1.71
diff -u -r1.71 apr_network_io.h
--- include/apr_network_io.h 2000/11/16 14:48:50 1.71
+++ include/apr_network_io.h 2000/11/16 15:42:14
@@ -265,7 +265,7 @@
* APR assumes that the sockaddr_in in the apr_socket is
* completely filled out.
*/
-apr_status_t apr_connect(apr_socket_t *sock, const char *hostname);
+apr_status_t apr_connect(apr_socket_t *sock, apr_sockaddr_t *sa);
/**
* Get name of a machine we are currently connected to.
@@ -275,6 +275,9 @@
*/
apr_status_t apr_get_hostname(char **name, apr_interface_e which, apr_socket_t *sock);
+apr_status_t apr_gethostbyname(apr_sockaddr_t **sa, const char *hostname,
+ apr_pool_t *p);
+
/**
* Get name of the current machine
* @param buf A buffer to store the hostname in.
@@ -434,6 +437,8 @@
* the port is already used, we won't find out about it here.
*/
apr_status_t apr_set_port(apr_socket_t *sock, apr_interface_e which, apr_port_t port);
+
+apr_status_t apr_set_sockaddr_port(apr_sockaddr_t *sa, apr_port_t port);
/**
* Return the port associated with a socket.
Index: network_io/unix/sa_common.c
===================================================================
RCS file: /home/cvspublic/apache-2.0/src/lib/apr/network_io/unix/sa_common.c,v
retrieving revision 1.6
diff -u -r1.6 sa_common.c
--- network_io/unix/sa_common.c 2000/11/16 14:48:49 1.6
+++ network_io/unix/sa_common.c 2000/11/16 15:42:15
@@ -78,6 +78,13 @@
return APR_SUCCESS;
}
+apr_status_t apr_set_sockaddr_port(apr_sockaddr_t *sa, apr_port_t port)
+{
+ /* XXX IPv6: assumes sin_port and sin6_port at same offset */
+ sa->sa.sin.sin_port = htons(port);
+ return APR_SUCCESS;
+}
+
apr_status_t apr_get_port(apr_port_t *port, apr_interface_e which, apr_socket_t *sock)
{
/* XXX IPv6: assumes sin_port and sin6_port at same offset */
@@ -171,3 +178,62 @@
return APR_SUCCESS;
}
+static void set_sockaddr_vars(apr_sockaddr_t *addr, int family)
+{
+ addr->sa.sin.sin_family = family;
+ addr->sa.sin.sin_family = family;
+
+ if (family == AF_INET) {
+ addr->sa_len = sizeof(struct sockaddr_in);
+ addr->addr_str_len = 16;
+ addr->ipaddr_ptr = &(addr->sa.sin.sin_addr);
+ addr->ipaddr_len = sizeof(struct in_addr);
+ }
+#if APR_HAVE_IPV6
+ else if (family == AF_INET6) {
+ addr->sa_len = sizeof(struct sockaddr_in6);
+ addr->addr_str_len = 46;
+ addr->ipaddr_ptr = &(addr->sa.sin6.sin6_addr);
+ addr->ipaddr_len = sizeof(struct in6_addr);
+ }
+#endif
+}
+
+apr_status_t apr_gethostbyname(apr_sockaddr_t **sa, const char *hostname, apr_pool_t *p)
+{
+ struct hostent *hp;
+
+ (*sa) = (apr_sockaddr_t *)apr_pcalloc(p, sizeof(apr_sockaddr_t));
+ if ((*sa) == NULL)
+ return APR_ENOMEM;
+ (*sa)->pool = p;
+ (*sa)->sa.sin.sin_family = AF_INET; /* we don't yet support IPv6 */
+ set_sockaddr_vars(*sa, (*sa)->sa.sin.sin_family);
+
+ if (hostname != NULL) {
+#ifndef GETHOSTBYNAME_HANDLES_NAS
+ if (*hostname >= '0' && *hostname <= '9' &&
+ strspn(hostname, "0123456789.") == strlen(hostname)) {
+ (*sa)->sa.sin.sin_addr.s_addr = inet_addr(hostname);
+ (*sa)->sa_len = sizeof(struct sockaddr_in);
+ }
+ else {
+#endif
+ hp = gethostbyname(hostname);
+
+ if (!hp) {
+ return (h_errno + APR_OS_START_SYSERR);
+ }
+
+ memcpy((char *)&(*sa)->sa.sin.sin_addr, hp->h_addr_list[0],
+ hp->h_length);
+ (*sa)->sa_len = sizeof(struct sockaddr_in);
+ (*sa)->ipaddr_len = hp->h_length;
+
+#ifndef GETHOSTBYNAME_HANDLES_NAS
+ }
+#endif
+ }
+ (*sa)->hostname = apr_pstrdup(p, hostname);
+ return APR_SUCCESS;
+}
Index: network_io/unix/sockets.c
===================================================================
RCS file: /home/cvspublic/apache-2.0/src/lib/apr/network_io/unix/sockets.c,v
retrieving revision 1.56
diff -u -r1.56 sockets.c
--- network_io/unix/sockets.c 2000/11/16 14:48:49 1.56
+++ network_io/unix/sockets.c 2000/11/16 15:42:16
@@ -226,57 +226,35 @@
return APR_SUCCESS;
}
-apr_status_t apr_connect(apr_socket_t *sock, const char *hostname)
+apr_status_t apr_connect(apr_socket_t *sock, apr_sockaddr_t *sa)
{
- struct hostent *hp;
-
if ((sock->socketdes < 0) || (!sock->remote_addr)) {
return APR_ENOTSOCK;
}
- if (hostname != NULL) {
-#ifndef GETHOSTBYNAME_HANDLES_NAS
- if (*hostname >= '0' && *hostname <= '9' &&
- strspn(hostname, "0123456789.") == strlen(hostname)) {
- sock->remote_addr->sa.sin.sin_addr.s_addr = inet_addr(hostname);
- }
- else {
-#endif
- hp = gethostbyname(hostname);
-
- if (!hp) {
- return (h_errno + APR_OS_START_SYSERR);
- }
-
- /* XXX IPv6: move name resolution out of this function */
- memcpy((char *)&sock->remote_addr->sa.sin.sin_addr, hp->h_addr_list[0],
- hp->h_length);
-
-#ifndef GETHOSTBYNAME_HANDLES_NAS
- }
-#endif
- }
- if ((connect(sock->socketdes,
- (const struct sockaddr *)&sock->remote_addr->sa.sin,
- sock->remote_addr->sa_len) < 0) &&
+ if ((connect(sock->socketdes,
+ (const struct sockaddr *)&sa->sa.sin,
+ sa->sa_len) < 0) &&
(errno != EINPROGRESS)) {
return errno;
}
else {
- /* XXX IPv6 */
+ sock->remote_addr = sa;
+ /* XXX IPv6 assumes sin_port and sin6_port at same offset */
if (sock->local_addr->sa.sin.sin_port == 0) {
/* connect() got us an ephemeral port */
sock->local_port_unknown = 1;
}
- /* XXX IPv6 */
- if (sock->local_addr->sa.sin.sin_addr.s_addr == 0) {
+ /* XXX IPv6 to be handled better later... */
+ if (sock->local_addr->sa.sin.sin_family == AF_INET6 ||
+ sock->local_addr->sa.sin.sin_addr.s_addr == 0) {
/* not bound to specific local interface; connect() had to assign
* one for the socket
*/
sock->local_interface_unknown = 1;
}
#ifndef HAVE_POLL
- sock->connected=1;
+ sock->connected=1;
#endif
return APR_SUCCESS;
}
Index: test/client.c
===================================================================
RCS file: /home/cvspublic/apache-2.0/src/lib/apr/test/client.c,v
retrieving revision 1.17
diff -u -r1.17 client.c
--- test/client.c 2000/11/09 15:01:35 1.17
+++ test/client.c 2000/11/16 15:42:16
@@ -73,6 +73,7 @@
char *dest = "127.0.0.1";
apr_port_t local_port, remote_port;
apr_interval_time_t read_timeout = -1;
+ apr_sockaddr_t *destsa;
setbuf(stdout, NULL);
if (argc > 1) {
@@ -115,8 +116,17 @@
fprintf(stdout, "OK\n");
}
+ fprintf(stdout,"\tClient: Making socket address...............");
+ if (apr_gethostbyname(&destsa, dest, context) != APR_SUCCESS){
+ apr_close_socket(sock);
+ fprintf(stdout, "Failed!\n");
+ fprintf(stdout, "Couldn't create a socket address structure for %s\n", dest);
+ exit(-1);
+ }
+ fprintf(stdout,"OK\n");
+
fprintf(stdout, "\tClient: Setting port for socket.......");
- if (apr_set_port(sock, APR_REMOTE, 8021) != APR_SUCCESS) {
+ if (apr_set_sockaddr_port(destsa, 8021) != APR_SUCCESS) {
apr_close_socket(sock);
fprintf(stderr, "Couldn't set the port correctly\n");
exit(-1);
@@ -125,7 +135,7 @@
fprintf(stdout, "\tClient: Connecting to socket.......");
- stat = apr_connect(sock, dest);
+ stat = apr_connect(sock, destsa);
if (stat != APR_SUCCESS) {
apr_close_socket(sock);
--
Jeff Trawick | trawick@ibm.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: OK...
Posted by Greg Stein <gs...@lyra.org>.
On Thu, Nov 16, 2000 at 01:49:33PM -0000, David Reid wrote:
>...
> > > 3. Before we can take this further, I guess we need to add the following
> > > though...
> >
> > I'll let others comment on this stuff. I'm much more concerned about
> > getting enough IPv6 enabled in APR so we can test it properly. Apache
> > needs can come a little later (for me, at least).
>
> Agreed in principal... However, we are about to beta Apache 2.0 (I did wanr
> that this change would screw things up a while back) so I'm not sure if the
> timing is good for stopping apache building. then again if we break it
> they'll have to fix it... :)
Doing the Right Thing is more important than maintaining a release schedule.
You guys should just make the changes. The release can be deferred a week if
need be.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: OK...
Posted by David Reid <dr...@jetnet.co.uk>.
[OK, I'm getting better at this Reply To All stuff...]
> > 1. Modify connect to take an apr_sockaddr_t instead of a hostname...
This
> > is a simple change so I'll make it sooner rather than later.
>
> yes...
OK, I've done some work on this but am getting an error now
Could not connect: Can't assign requested address (49)
I've got to drive to Suffolk in a short while so I'll post the patch at the
end of this message and let someone else figure out where I've screwed up!
:)
>
> > ??? Do we change apr_gethostbyname to return an apr_sockaddr_t?
>
> yes...
>
> > This would then give
> > apr_sockaddr_t *sa;
> > apr_gethostbyname(&sa, "hostname", pool);
> > apr_connect(sock, sa);
>
> We need to add more arguments: address family for sure and possibly
> flags. Look at getaddrinfo(). And do we want to add an optional
> service name/port number parameter? That would build the complete
> sockaddr for us.
OK. Care to suggest the definition?
>
> > 2. Are we agreed on Jeff's suggestions of
> > Add apr_pool_t * to apr_sockaddr_t.
> > apr_status_t apr_get_address(char **hostname, apr_interface_e which,
> > apr_socket_t *sock);
> > apr_status_t apr_get_nas(char **addr, apr_sockaddr_t *sa);
> > These are new additions sos houldn't interfere with any existing
> > code.
>
> I am.
Good.
>
> In addition to the changes you mentioned, I see apr_create_socket() as
> extremely important in the short run and I think we should think about
> apr_bind() working like apr_connect() (in other words, taking an
> apr_sockaddr_t). That makes sense when the user has told us the local
> interface address and we have to resolve it anyway. We have to keep
> it from being painful when we just have the port number.
OK, so again care to suggest the API definitions?
>
> > 3. Before we can take this further, I guess we need to add the following
> > though...
>
> I'll let others comment on this stuff. I'm much more concerned about
> getting enough IPv6 enabled in APR so we can test it properly. Apache
> needs can come a little later (for me, at least).
Agreed in principal... However, we are about to beta Apache 2.0 (I did wanr
that this change would screw things up a while back) so I'm not sure if the
timing is good for stopping apache building. then again if we break it
they'll have to fix it... :)
david
Index: include/apr_network_io.h
===================================================================
RCS file: /home/cvs/apache-2.0/src/lib/apr/include/apr_network_io.h,v
retrieving revision 1.70
diff -u -r1.70 apr_network_io.h
--- include/apr_network_io.h 2000/11/16 01:51:33 1.70
+++ include/apr_network_io.h 2000/11/16 13:40:54
@@ -255,7 +255,7 @@
* APR assumes that the sockaddr_in in the apr_socket is
* completely filled out.
*/
-apr_status_t apr_connect(apr_socket_t *sock, const char *hostname);
+apr_status_t apr_connect(apr_socket_t *sock, apr_sockaddr_t *sa);
/**
* Get name of a machine we are currently connected to.
+apr_status_t apr_gethostbyname(apr_sockaddr_t **sa, const char *hostname,
apr_pool_t *p);
Modify apr_connect...
apr_status_t apr_connect(apr_socket_t *sock, apr_sockaddr_t *sa)
{
if ((sock->socketdes < 0) || (!sock->remote_addr)) {
return APR_ENOTSOCK;
}
if ((connect(sock->socketdes,
(const struct sockaddr *)&sa->sa.sin,
sa->sa_len) < 0) &&
(errno != EINPROGRESS)) {
return errno;
}
else {
sock->remote_addr = sa;
/* XXX IPv6 */
if (sock->local_addr->sa.sin.sin_port == 0) {
/* connect() got us an ephemeral port */
sock->local_port_unknown = 1;
}
/* XXX IPv6 */
if (sock->local_addr->sa.sin.sin_addr.s_addr == 0) {
/* not bound to specific local interface; connect() had to
assign
* one for the socket
*/
sock->local_interface_unknown = 1;
}
#ifndef HAVE_POLL
sock->connected=1;
#endif
return APR_SUCCESS;
}
}
This into sa_common.c
apr_status_t apr_gethostbyname(apr_sockaddr_t **sa, const char *hostname,
apr_pool_t *p)
{
struct hostent *hp;
(*sa) = (apr_sockaddr_t *)apr_pcalloc(p, sizeof(apr_sockaddr_t));
if ((*sa) == NULL)
return APR_ENOMEM;
(*sa)->pool = p;
(*sa)->sa.sin.sin_family = AF_INET; /* we don't yet support IPv6 */
if (hostname != NULL) {
#ifndef GETHOSTBYNAME_HANDLES_NAS
if (*hostname >= '0' && *hostname <= '9' &&
strspn(hostname, "0123456789.") == strlen(hostname)) {
(*sa)->sa.sin.sin_addr.s_addr = inet_addr(hostname);
(*sa)->sa_len = sizeof(struct sockaddr_in);
}
else {
#endif
hp = gethostbyname(hostname);
if (!hp) {
return (h_errno + APR_OS_START_SYSERR);
}
/* XXX IPv6: move name resolution out of this function */
memcpy((char *)&(*sa)->sa.sin.sin_addr, hp->h_addr_list[0],
hp->h_length);
(*sa)->sa_len = sizeof(struct sockaddr_in);
(*sa)->ipaddr_len = hp->h_length;
#ifndef GETHOSTBYNAME_HANDLES_NAS
}
#endif
}
(*sa)->hostname = apr_pstrdup(p, hostname);
return APR_SUCCESS;
}
Index: test/client.c
===================================================================
RCS file: /home/cvs/apache-2.0/src/lib/apr/test/client.c,v
retrieving revision 1.17
diff -u -r1.17 client.c
--- test/client.c 2000/11/09 15:01:35 1.17
+++ test/client.c 2000/11/16 13:36:45
@@ -73,6 +73,7 @@
char *dest = "127.0.0.1";
apr_port_t local_port, remote_port;
apr_interval_time_t read_timeout = -1;
+ apr_sockaddr_t *destsa;
setbuf(stdout, NULL);
if (argc > 1) {
@@ -123,9 +124,18 @@
}
fprintf(stdout, "OK\n");
+ fprintf(stdout,"\tClient: Making socket address...............");
+ if (apr_gethostbyname(&destsa, dest, context) != APR_SUCCESS){
+ apr_close_socket(sock);
+ fprintf(stdout, "Failed!\n");
+ fprintf(stdout, "Couldn't create a socket address structure for
%s\n", dest);
+ exit(-1);
+ }
+ fprintf(stdout,"OK\n");
+
fprintf(stdout, "\tClient: Connecting to socket.......");
- stat = apr_connect(sock, dest);
+ stat = apr_connect(sock, destsa);
if (stat != APR_SUCCESS) {
apr_close_socket(sock);
Re: OK...
Posted by Jeff Trawick <tr...@bellsouth.net>.
Here are some idioms I see in other IPv6-enabled code which I think
should be supported in APR. Part of this has a bearing on my comments
to you post below.
1) stream socket server
a. (normal) get me a darn socket, I don't care which kind
API: apr_create_socket(, AF_UNSPEC, SOCK_STREAM, ...)
(AF_UNSPEC could be anything to tell APR we don't
care what kind of socket)
APR gets AF_INET6 if possible, falls back to AF_INET
b. hmmm... user told me to bind to a certain local interface
address... I need to resolve the address (could be hostname
or numeric address string -- I shouldn't have to care) then
get the right kind of socket
API: apr_gethostbyname(&sa, hostname-or-nas, AF_UNSPEC, flags)
set-port-in-sockaddr
apr_create_socket(&sock,
whatever-family-chosen-in-apr_gethostbyname,
SOCK_STREAM, ...)
apr_bind(sock, sa)
2) stream socket client:
a. (normal) user gave me a client hostname (or numeric address
string -- I shouldn't have to care)... I need to resolve the
address then get the right kind of socket
API: apr_gethostbyname(&sa, hostname-or-nas, AF_UNSPEC,
flags)
set-port-in-sockaddr
apr_create_socket(&sock,
whatever-family-chosen-in-apr_gethostbyname,
SOCK_STREAM, ...)
apr_connect(sock, sa)
b. same as 2a, but user told me to use a certain address family
(e.g., via "-4" or "-6" command-line parameter); this is
needed when IPv4 and IPv6 network connectivity is different...
you want to talk to host xyz with both types of addresses, but
6bone is down at the moment so you need to force IPv4
API: same as 2a, except you pass AF_INET or AF_INET6
instead of AF_UNSPEC to apr_gethostbyname()
"David Reid" <dr...@jetnet.co.uk> writes:
> 1. Modify connect to take an apr_sockaddr_t instead of a hostname... This
> is a simple change so I'll make it sooner rather than later.
yes...
> ??? Do we change apr_gethostbyname to return an apr_sockaddr_t?
yes...
> This would then give
> apr_sockaddr_t *sa;
> apr_gethostbyname(&sa, "hostname", pool);
> apr_connect(sock, sa);
We need to add more arguments: address family for sure and possibly
flags. Look at getaddrinfo(). And do we want to add an optional
service name/port number parameter? That would build the complete
sockaddr for us.
> 2. Are we agreed on Jeff's suggestions of
> Add apr_pool_t * to apr_sockaddr_t.
> apr_status_t apr_get_address(char **hostname, apr_interface_e which,
> apr_socket_t *sock);
> apr_status_t apr_get_nas(char **addr, apr_sockaddr_t *sa);
> These are new additions sos houldn't interfere with any existing
> code.
I am.
In addition to the changes you mentioned, I see apr_create_socket() as
extremely important in the short run and I think we should think about
apr_bind() working like apr_connect() (in other words, taking an
apr_sockaddr_t). That makes sense when the user has told us the local
interface address and we have to resolve it anyway. We have to keep
it from being painful when we just have the port number.
> 3. Before we can take this further, I guess we need to add the following
> though...
I'll let others comment on this stuff. I'm much more concerned about
getting enough IPv6 enabled in APR so we can test it properly. Apache
needs can come a little later (for me, at least).
--
Jeff Trawick | trawick@ibm.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...