You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by David Reid <dr...@jetnet.co.uk> on 2000/11/10 02:22:30 UTC

apr_sockaddr_t

Folks,

Next step in APR is to abstract out the sockaddr's.  We don't use the raw
struct in many places, but we need to remove the remaining references.  Big
problem will be http_vhost...

This patch adds a new apr_sockaddr_t type.  It has IPv6 support built into
it as it makes to start adding it now...  It also adds an enumeration of the
socket types we'll know how to create...

Continuing the theme of softly, softly, I'd like to commit this and then
wait to see if anyone ahs any problems (Bill & Bill??).  If the structure
isn't platform safe I'll move it to the networkio.h header files.

Once we're happy with the structure I'd like to start moving it into the
code, but until I get a chance to go through http_vhost it won't be possible
to do too much :(

Ideally I'd like to get this in as I suspect it'll break a fair bit of code
in the short term...

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.65
diff -u -r1.65 apr_network_io.h
--- include/apr_network_io.h    2000/11/10 00:58:25     1.65
+++ include/apr_network_io.h    2000/11/10 01:16:49
@@ -119,6 +119,12 @@
     APR_REMOTE
 } apr_interface_e;

+/* Enum of the types of socket we can create... */
+typedef enum {
+    APR_TCP_V4,
+    APR_TCP_V6
+} apr_sockettype_e;
+
 /* I guess not everybody uses inet_addr.  This defines apr_inet_addr
  * appropriately.
  */
@@ -136,6 +142,29 @@

 /* use apr_uint16_t just in case some system has a short that isn't 16
bits... */
 typedef apr_uint16_t            apr_port_t;
+
+/* we're going to roll our own sockaddr type as we want to make sure
+ * we have protocol independance for APR...
+ *
+ * It's defined here as I think it should all be platform safe...
+ */
+typedef struct {
+    apr_pool_t *cont;              /* The pool to use... */
+    char *hostname;                /* The hostname */
+    char *port_str;                /* String representation of port */
+    apr_port_t port;               /* numeric port */
+    union {
+        struct sockaddr_in sin;    /* IPv4 sockaddr structure */
+#if APR_HAVE_IPV6
+        struct sockaddr_in6 sin6;  /* IPv6 sockaddr structure */
+#endif
+    } sa;
+    apr_socklen_t sa_len;          /* How big is the sockaddr we're using?
*/
+    int addr_len;                  /* How big should the address buffer be?
+                                    * 16 for v4 or 48 for v6
+                                    * used in inet_ntop...
+                                    */
+} apr_sockaddr_t;

 #if APR_HAS_SENDFILE
 /* Define flags passed in on apr_sendfile() */



Re: apr_sockaddr_t

Posted by Jun Kuriyama <ku...@imgsrc.co.jp>.
> > the largest UDP or TCP port is 65535, so max string length is 5 decimal
> > digits + terminating '\0'
> 
> Yes, but what about for the named ports?  If we have "telnet\0" that's 7,
> and is that the biggest?  I was thinking of port_str being used for known
> ports as well as string representations (maybe I should change the comment?)
> and I wasn't sure what the longest would be.  I see there is a define of
> NI_MAXSERV at 32, so would 32 be a suitable value and do we want to go that
> large?

I think NI_MAXSERV is appropriate one (if that is defined in target
OS).

NI_MAXSERV and NI_MAXHOST seems to be defined if that OS supports
RFC2553.


-- 
Jun Kuriyama <ku...@imgsrc.co.jp> // IMG SRC, Inc.
             <ku...@FreeBSD.org> // FreeBSD Project

Re: apr_sockaddr_t

Posted by David Reid <dr...@jetnet.co.uk>.
OK.  I can do that.  I'll change the comment :)

david
----- Original Message -----
From: "Jeff Trawick" <tr...@bellsouth.net>
To: <ne...@apache.org>
Sent: Friday, November 10, 2000 3:22 PM
Subject: Re: apr_sockaddr_t


> "David Reid" <dr...@jetnet.co.uk> writes:
>
> > > > >   Consider reserving space for the port string directly in the
> > > > >   apr_sockaddr_t:
> > > > >
> > > > >       char port_str[6];
> > > >
> > > > This makes sense, but I wasn't sure what the biggest string we could
> > have
> > > > would be?  Is 6 big enough?
> > >
> > > the largest UDP or TCP port is 65535, so max string length is 5
decimal
> > > digits + terminating '\0'
> >
> > Yes, but what about for the named ports?
>
> I didn't realize this was for service names.  Forget my comments about
> putting the string inline...
>
> Maybe you want to rename the field?  The parameter to getaddrinfo()
> which has the same usage (i.e., a service name if available or a
> decimal port number string otherwise) is called servname.
>
> --
> 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: apr_sockaddr_t

Posted by Jeff Trawick <tr...@bellsouth.net>.
"David Reid" <dr...@jetnet.co.uk> writes:

> > > >   Consider reserving space for the port string directly in the
> > > >   apr_sockaddr_t:
> > > >
> > > >       char port_str[6];
> > >
> > > This makes sense, but I wasn't sure what the biggest string we could
> have
> > > would be?  Is 6 big enough?
> >
> > the largest UDP or TCP port is 65535, so max string length is 5 decimal
> > digits + terminating '\0'
> 
> Yes, but what about for the named ports?  

I didn't realize this was for service names.  Forget my comments about
putting the string inline... 

Maybe you want to rename the field?  The parameter to getaddrinfo()
which has the same usage (i.e., a service name if available or a
decimal port number string otherwise) is called servname.

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

Posted by David Reid <dr...@jetnet.co.uk>.
> > >   Consider reserving space for the port string directly in the
> > >   apr_sockaddr_t:
> > >
> > >       char port_str[6];
> >
> > This makes sense, but I wasn't sure what the biggest string we could
have
> > would be?  Is 6 big enough?
>
> the largest UDP or TCP port is 65535, so max string length is 5 decimal
> digits + terminating '\0'

Yes, but what about for the named ports?  If we have "telnet\0" that's 7,
and is that the biggest?  I was thinking of port_str being used for known
ports as well as string representations (maybe I should change the comment?)
and I wasn't sure what the longest would be.  I see there is a define of
NI_MAXSERV at 32, so would 32 be a suitable value and do we want to go that
large?

david



Re: apr_sockaddr_t

Posted by Jeff Trawick <tr...@bellsouth.net>.
"David Reid" <dr...@jetnet.co.uk> writes:

> >   Consider reserving space for the port string directly in the
> >   apr_sockaddr_t:
> >
> >       char port_str[6];
> 
> This makes sense, but I wasn't sure what the biggest string we could have
> would be?  Is 6 big enough?

the largest UDP or TCP port is 65535, so max string length is 5 decimal
digits + terminating '\0'

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

Posted by David Reid <dr...@jetnet.co.uk>.
>
> Sure (+1)...
>
> Some minor suggestions...
>
> >+    apr_pool_t *cont;              /* The pool to use... */
>
>   Consider renaming field "cont" to "p" or "pool"
>

OK.  the use of cont was to fit in with other structures, but hey, we can be
different.

> >+    int addr_len;                  /* How big should the address buffer
be?
> >+                                    * 16 for v4 or 48 for v6
> >+                                    * used in inet_ntop...
> >+                                    */
>
>   Consider renaming addr_len to addr_str_len

Yes, this also makes sense.
>
> >+    char *port_str;                /* String representation of port
> >*/
>
>   Consider reserving space for the port string directly in the
>   apr_sockaddr_t:
>
>       char port_str[6];

This makes sense, but I wasn't sure what the biggest string we could have
would be?  Is 6 big enough?

david


Re: apr_sockaddr_t

Posted by Jeff Trawick <tr...@bellsouth.net>.
Tony Finch <do...@dotat.at> writes:

> Jeff Trawick <tr...@bellsouth.net> wrote:
> >
> >>+    int addr_len;                  /* How big should the address buffer be?
> >>+                                    * 16 for v4 or 48 for v6
> >>+                                    * used in inet_ntop...
> >>+                                    */
> >
> >  Consider renaming addr_len to addr_str_len
> 
> What is "str" short for? Surely not "string" as in the other names in
> the structure, because that would be wrong.

"str" stands for "string" in this case...  David wants a field to
specify how long the numeric address string can be.

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

Posted by David Reid <dr...@jetnet.co.uk>.
The field is there so that if we need to call inet_ntop then we have the
length of the string buffer that we should be passing in.  This seemed like
a useful thing to do at the time we decide what family the sockaddr is in...
It can be removed if people really object.

david

----- Original Message -----
From: "Tony Finch" <do...@dotat.at>
To: <ne...@apache.org>
Sent: Monday, November 13, 2000 9:37 PM
Subject: Re: apr_sockaddr_t


> Jeff Trawick <tr...@bellsouth.net> wrote:
> >
> >>+    int addr_len;                  /* How big should the address buffer
be?
> >>+                                    * 16 for v4 or 48 for v6
> >>+                                    * used in inet_ntop...
> >>+                                    */
> >
> >  Consider renaming addr_len to addr_str_len
>
> What is "str" short for? Surely not "string" as in the other names in
> the structure, because that would be wrong.
>
> Tony.
> --
> en oeccget g mtcaa    f.a.n.finch
> v spdlkishrhtewe y    dot@dotat.at
> eatp o v eiti i d.    fanf@covalent.net
>


Re: apr_sockaddr_t

Posted by Tony Finch <do...@dotat.at>.
Jeff Trawick <tr...@bellsouth.net> wrote:
>
>>+    int addr_len;                  /* How big should the address buffer be?
>>+                                    * 16 for v4 or 48 for v6
>>+                                    * used in inet_ntop...
>>+                                    */
>
>  Consider renaming addr_len to addr_str_len

What is "str" short for? Surely not "string" as in the other names in
the structure, because that would be wrong.

Tony.
-- 
en oeccget g mtcaa    f.a.n.finch
v spdlkishrhtewe y    dot@dotat.at
eatp o v eiti i d.    fanf@covalent.net

Re: apr_sockaddr_t

Posted by Jeff Trawick <tr...@bellsouth.net>.
"David Reid" <dr...@jetnet.co.uk> writes:

> Ahh, yes you're right on the sin_family...
> 
> > 
> > perhaps the offset to the port and a macro to encapsulate the use of
> > it would be useful...  we don't need the pointer to the family...
> 
> Can I take that as a +1 for adding the structure?

Sure (+1)...  

Some minor suggestions...

>+    apr_pool_t *cont;              /* The pool to use... */

  Consider renaming field "cont" to "p" or "pool"

>+    int addr_len;                  /* How big should the address buffer be?
>+                                    * 16 for v4 or 48 for v6
>+                                    * used in inet_ntop...
>+                                    */

  Consider renaming addr_len to addr_str_len

>+    char *port_str;                /* String representation of port
>*/

  Consider reserving space for the port string directly in the
  apr_sockaddr_t:

      char port_str[6];

  (think of padding added by some compilers: it may not take more
  space in the structure and it simplifies the code a bit)

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

Posted by David Reid <dr...@jetnet.co.uk>.
Ahh, yes you're right on the sin_family...

> 
> perhaps the offset to the port and a macro to encapsulate the use of
> it would be useful...  we don't need the pointer to the family...

Can I take that as a +1 for adding the structure?

david



Re: apr_sockaddr_t

Posted by Jeff Trawick <tr...@bellsouth.net>.
"David Reid" <dr...@jetnet.co.uk> writes:

> Of course we should probably also have a family flag so we can check it...
> We can't use the sockaddr structures as we don't know which one we're
> looking at without knowing the family!

no, the family is always in the same place just to solve this
problem... simply look at sin_family...

> 
> I realise we don't currently use some of these and the functions to do some
> of what I've described don't yet exist, but they will at some point and so
> we should plan on making the structure as useful as we can.
> 
> In some ways I'd also like to add a set of pointers to the structure.  These
> would be pointers to the start of the family, sockaddr structure and port
> within wither the sin or sin6 structure.  By setting these when we know if
> we're v4 or v6 we can reduce the number of times we have to check, and so
> coding becomes much cleaner.

perhaps the offset to the port and a macro to encapsulate the use of
it would be useful...  we don't need the pointer to the family...

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

Posted by David Reid <dr...@jetnet.co.uk>.
The hostname is there as if we pass in a hostname and ask for an
apr_sockaddr_t to be returned we might as well keep the hostname so should
we ask for the hostname for the apr_sockaddr_t we don't have any costly
calls to get it, we can just return it.
The string for the port?  It's there because if we ask for a port by name,
say 'http' or 'telnet' we can again keep that and have the port number as
well.
Why the addr_len?  When we use functions that need to know how long the
string buffer should be we shouldn't be determining at that point of we're
IPv4 or IPv6, we should just be able to grab the length and use it.

Of course we should probably also have a family flag so we can check it...
We can't use the sockaddr structures as we don't know which one we're
looking at without knowing the family!

I realise we don't currently use some of these and the functions to do some
of what I've described don't yet exist, but they will at some point and so
we should plan on making the structure as useful as we can.

In some ways I'd also like to add a set of pointers to the structure.  These
would be pointers to the start of the family, sockaddr structure and port
within wither the sin or sin6 structure.  By setting these when we know if
we're v4 or v6 we can reduce the number of times we have to check, and so
coding becomes much cleaner.

david

----- Original Message -----
From: "Tony Finch" <do...@dotat.at>
To: <ne...@apache.org>
Sent: Friday, November 10, 2000 2:50 AM
Subject: Re: apr_sockaddr_t


>
> David Reid <dr...@jetnet.co.uk> wrote:
> >+
> >+/* we're going to roll our own sockaddr type as we want to make sure
> >+ * we have protocol independance for APR...
> >+ *
> >+ * It's defined here as I think it should all be platform safe...
> >+ */
> >+typedef struct {
> >+    apr_pool_t *cont;              /* The pool to use... */
> >+    char *hostname;                /* The hostname */
> >+    char *port_str;                /* String representation of port */
> >+    apr_port_t port;               /* numeric port */
> >+    union {
> >+        struct sockaddr_in sin;    /* IPv4 sockaddr structure */
> >+#if APR_HAVE_IPV6
> >+        struct sockaddr_in6 sin6;  /* IPv6 sockaddr structure */
> >+#endif
> >+    } sa;
> >+    apr_socklen_t sa_len;          /* How big is the sockaddr we're
using?
> >*/
> >+    int addr_len;                  /* How big should the address buffer
be?
> >+                                    * 16 for v4 or 48 for v6
> >+                                    * used in inet_ntop...
> >+                                    */
> >+} apr_sockaddr_t;
>
> That looks absurdly baroque to me. Why is the hostname there? I think
> name <-> address translations should be handled at a higher level than
> sockaddrs. Why duplicate the port?
>
> Tony.
> --
> en oeccget g mtcaa    f.a.n.finch
> v spdlkishrhtewe y    dot@dotat.at
> eatp o v eiti i d.    fanf@covalent.net
>


Re: apr_sockaddr_t

Posted by Tony Finch <do...@dotat.at>.
David Reid <dr...@jetnet.co.uk> wrote:
>+
>+/* we're going to roll our own sockaddr type as we want to make sure
>+ * we have protocol independance for APR...
>+ *
>+ * It's defined here as I think it should all be platform safe...
>+ */
>+typedef struct {
>+    apr_pool_t *cont;              /* The pool to use... */
>+    char *hostname;                /* The hostname */
>+    char *port_str;                /* String representation of port */
>+    apr_port_t port;               /* numeric port */
>+    union {
>+        struct sockaddr_in sin;    /* IPv4 sockaddr structure */
>+#if APR_HAVE_IPV6
>+        struct sockaddr_in6 sin6;  /* IPv6 sockaddr structure */
>+#endif
>+    } sa;
>+    apr_socklen_t sa_len;          /* How big is the sockaddr we're using?
>*/
>+    int addr_len;                  /* How big should the address buffer be?
>+                                    * 16 for v4 or 48 for v6
>+                                    * used in inet_ntop...
>+                                    */
>+} apr_sockaddr_t;

That looks absurdly baroque to me. Why is the hostname there? I think
name <-> address translations should be handled at a higher level than
sockaddrs. Why duplicate the port?

Tony.
-- 
en oeccget g mtcaa    f.a.n.finch
v spdlkishrhtewe y    dot@dotat.at
eatp o v eiti i d.    fanf@covalent.net

Re: apr_sockaddr_t

Posted by rb...@covalent.net.
++1.

Ryan

> 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.65
> diff -u -r1.65 apr_network_io.h
> --- include/apr_network_io.h    2000/11/10 00:58:25     1.65
> +++ include/apr_network_io.h    2000/11/10 01:16:49
> @@ -119,6 +119,12 @@
>      APR_REMOTE
>  } apr_interface_e;
> 
> +/* Enum of the types of socket we can create... */
> +typedef enum {
> +    APR_TCP_V4,
> +    APR_TCP_V6
> +} apr_sockettype_e;
> +
>  /* I guess not everybody uses inet_addr.  This defines apr_inet_addr
>   * appropriately.
>   */
> @@ -136,6 +142,29 @@
> 
>  /* use apr_uint16_t just in case some system has a short that isn't 16
> bits... */
>  typedef apr_uint16_t            apr_port_t;
> +
> +/* we're going to roll our own sockaddr type as we want to make sure
> + * we have protocol independance for APR...
> + *
> + * It's defined here as I think it should all be platform safe...
> + */
> +typedef struct {
> +    apr_pool_t *cont;              /* The pool to use... */
> +    char *hostname;                /* The hostname */
> +    char *port_str;                /* String representation of port */
> +    apr_port_t port;               /* numeric port */
> +    union {
> +        struct sockaddr_in sin;    /* IPv4 sockaddr structure */
> +#if APR_HAVE_IPV6
> +        struct sockaddr_in6 sin6;  /* IPv6 sockaddr structure */
> +#endif
> +    } sa;
> +    apr_socklen_t sa_len;          /* How big is the sockaddr we're using?
> */
> +    int addr_len;                  /* How big should the address buffer be?
> +                                    * 16 for v4 or 48 for v6
> +                                    * used in inet_ntop...
> +                                    */
> +} apr_sockaddr_t;
> 
>  #if APR_HAS_SENDFILE
>  /* Define flags passed in on apr_sendfile() */
> 
> 
> 


_______________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------


RE: apr_sockaddr_t

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Just commit, I can be sure this works in an hour.

> -----Original Message-----
> From: David Reid [mailto:dreid@jetnet.co.uk]
> Sent: Thursday, November 09, 2000 7:23 PM
> To: Apache (new-httpd)
> Subject: apr_sockaddr_t
> 
> 
> Folks,
> 
> Next step in APR is to abstract out the sockaddr's.  We don't 
> use the raw
> struct in many places, but we need to remove the remaining 
> references.  Big
> problem will be http_vhost...
> 
> This patch adds a new apr_sockaddr_t type.  It has IPv6 
> support built into
> it as it makes to start adding it now...  It also adds an 
> enumeration of the
> socket types we'll know how to create...
> 
> Continuing the theme of softly, softly, I'd like to commit 
> this and then
> wait to see if anyone ahs any problems (Bill & Bill??).  If 
> the structure
> isn't platform safe I'll move it to the networkio.h header files.
> 
> Once we're happy with the structure I'd like to start moving 
> it into the
> code, but until I get a chance to go through http_vhost it 
> won't be possible
> to do too much :(
> 
> Ideally I'd like to get this in as I suspect it'll break a 
> fair bit of code
> in the short term...
> 
> 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.65
> diff -u -r1.65 apr_network_io.h
> --- include/apr_network_io.h    2000/11/10 00:58:25     1.65
> +++ include/apr_network_io.h    2000/11/10 01:16:49
> @@ -119,6 +119,12 @@
>      APR_REMOTE
>  } apr_interface_e;
> 
> +/* Enum of the types of socket we can create... */
> +typedef enum {
> +    APR_TCP_V4,
> +    APR_TCP_V6
> +} apr_sockettype_e;
> +
>  /* I guess not everybody uses inet_addr.  This defines apr_inet_addr
>   * appropriately.
>   */
> @@ -136,6 +142,29 @@
> 
>  /* use apr_uint16_t just in case some system has a short 
> that isn't 16
> bits... */
>  typedef apr_uint16_t            apr_port_t;
> +
> +/* we're going to roll our own sockaddr type as we want to make sure
> + * we have protocol independance for APR...
> + *
> + * It's defined here as I think it should all be platform safe...
> + */
> +typedef struct {
> +    apr_pool_t *cont;              /* The pool to use... */
> +    char *hostname;                /* The hostname */
> +    char *port_str;                /* String representation 
> of port */
> +    apr_port_t port;               /* numeric port */
> +    union {
> +        struct sockaddr_in sin;    /* IPv4 sockaddr structure */
> +#if APR_HAVE_IPV6
> +        struct sockaddr_in6 sin6;  /* IPv6 sockaddr structure */
> +#endif
> +    } sa;
> +    apr_socklen_t sa_len;          /* How big is the 
> sockaddr we're using?
> */
> +    int addr_len;                  /* How big should the 
> address buffer be?
> +                                    * 16 for v4 or 48 for v6
> +                                    * used in inet_ntop...
> +                                    */
> +} apr_sockaddr_t;
> 
>  #if APR_HAS_SENDFILE
>  /* Define flags passed in on apr_sendfile() */
> 
> 
>