You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Ryan Morgan <rm...@covalent.net> on 2002/04/05 23:29:16 UTC

[patch] apr_sockaddr_port_get broken

The cleanup of the unix networking code a while back has broken 
apr_sockaddr_port_get when 0 is specifed as the port to bind() to.  This
is because we now grab the port from sockaddr->port rather than the
actual sockaddr.  (sockaddr->port is never updated after 
apr_sockaddr_port_set is called).

A simple patch is:

Index: sa_common.c
===================================================================
RCS file: /home/cvspublic/apr/network_io/unix/sa_common.c,v
retrieving revision 1.53
diff -u -r1.53 sa_common.c
--- sa_common.c 28 Mar 2002 18:17:02 -0000      1.53
+++ sa_common.c 5 Apr 2002 21:20:58 -0000
@@ -139,7 +139,7 @@
 APR_DECLARE(apr_status_t) apr_sockaddr_port_get(apr_port_t *port,
                                        apr_sockaddr_t *sockaddr)
 {
-    *port = sockaddr->port;
+    *port = ntohs(sockaddr->sa.sin.sin_port);
     return APR_SUCCESS;
 }

But i assume this is wrong, since this just reverts the changes that
Jeff made a week or two ago.

-Ryan

Re: [patch] apr_sockaddr_port_get broken

Posted by Jeff Trawick <tr...@attglobal.net>.
Ryan Morgan <rm...@covalent.net> writes:

> On Fri, Apr 05, 2002 at 04:42:33PM -0500, Jeff Trawick wrote:
> > Ryan Morgan <rm...@covalent.net> writes:
> > 
> > > The cleanup of the unix networking code a while back has broken 
> > > apr_sockaddr_port_get when 0 is specifed as the port to bind() to.  This
> > > is because we now grab the port from sockaddr->port rather than the
> > > actual sockaddr.  (sockaddr->port is never updated after 
> > > apr_sockaddr_port_set is called).
> > 
> > I would think that the better fix is to update sockaddr->port.
> > 
> > Can you try this instead (need win32 and os/2 flavors as well :( )?
> 
> Ah, much better.  That fixes the problem for me.  (And should fix win32
> and os/2 since they all use the same apr_socket_addr_get, correct?)

These few lines of code are OS-specific, unfortunately.  But there are
two versions, not three.  It turns out that OS/2 uses the Unix code
for this.

I'll commit the Unix code you tested and the identical Win32 code in
just a sec.

-- 
Jeff Trawick | trawick@attglobal.net
Born in Roswell... married an alien...

Re: [patch] apr_sockaddr_port_get broken

Posted by Ryan Morgan <rm...@covalent.net>.
On Fri, Apr 05, 2002 at 04:42:33PM -0500, Jeff Trawick wrote:
> Ryan Morgan <rm...@covalent.net> writes:
> 
> > The cleanup of the unix networking code a while back has broken 
> > apr_sockaddr_port_get when 0 is specifed as the port to bind() to.  This
> > is because we now grab the port from sockaddr->port rather than the
> > actual sockaddr.  (sockaddr->port is never updated after 
> > apr_sockaddr_port_set is called).
> 
> I would think that the better fix is to update sockaddr->port.
> 
> Can you try this instead (need win32 and os/2 flavors as well :( )?

Ah, much better.  That fixes the problem for me.  (And should fix win32
and os/2 since they all use the same apr_socket_addr_get, correct?)

-Ryan

> 
> (I can't play for a few hours.)
> 
> Index: srclib/apr/network_io/unix/sockaddr.c
> ===================================================================
> RCS file: /home/cvs/apr/network_io/unix/sockaddr.c,v
> retrieving revision 1.28
> diff -u -r1.28 sockaddr.c
> --- srclib/apr/network_io/unix/sockaddr.c       13 Mar 2002 20:39:25
> -0000      1.28
> +++ srclib/apr/network_io/unix/sockaddr.c       5 Apr 2002 21:44:21
> -0000
> @@ -64,6 +64,8 @@
>      }
>      else {
>          sock->local_port_unknown = sock->local_interface_unknown = 0;
> +        /* XXX assumes sin_port and sin6_port at same offset */
> +        sock->local_addr->port = ntohs(sock->local_addr->sa.sin.sin_port);
>          return APR_SUCCESS;
>      }
>  }
> 
> -- 
> Jeff Trawick | trawick@attglobal.net
> Born in Roswell... married an alien...

Re: [patch] apr_sockaddr_port_get broken

Posted by Jeff Trawick <tr...@attglobal.net>.
Ryan Morgan <rm...@covalent.net> writes:

> The cleanup of the unix networking code a while back has broken 
> apr_sockaddr_port_get when 0 is specifed as the port to bind() to.  This
> is because we now grab the port from sockaddr->port rather than the
> actual sockaddr.  (sockaddr->port is never updated after 
> apr_sockaddr_port_set is called).

I would think that the better fix is to update sockaddr->port.

Can you try this instead (need win32 and os/2 flavors as well :( )?

(I can't play for a few hours.)

Index: srclib/apr/network_io/unix/sockaddr.c
===================================================================
RCS file: /home/cvs/apr/network_io/unix/sockaddr.c,v
retrieving revision 1.28
diff -u -r1.28 sockaddr.c
--- srclib/apr/network_io/unix/sockaddr.c       13 Mar 2002 20:39:25
-0000      1.28
+++ srclib/apr/network_io/unix/sockaddr.c       5 Apr 2002 21:44:21
-0000
@@ -64,6 +64,8 @@
     }
     else {
         sock->local_port_unknown = sock->local_interface_unknown = 0;
+        /* XXX assumes sin_port and sin6_port at same offset */
+        sock->local_addr->port = ntohs(sock->local_addr->sa.sin.sin_port);
         return APR_SUCCESS;
     }
 }

-- 
Jeff Trawick | trawick@attglobal.net
Born in Roswell... married an alien...