You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by GitBox <gi...@apache.org> on 2021/05/12 17:14:45 UTC
[GitHub] [trafficserver] SolidWallOfCode opened a new pull request #7821: Add method to write an IpAddr value to a sockaddr.
SolidWallOfCode opened a new pull request #7821:
URL: https://github.com/apache/trafficserver/pull/7821
This method is needed for HostDB restructuring so that an address stored in an `IpAddr` can be easily written to a `sockaddr`. It's already possible to extract an `IpAddr` from a `sockaddr` and should be easy to do the reverse.
I'm not completely thrilled with the method name, so I'm open bikeshedding on that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] SolidWallOfCode removed a comment on pull request #7821: Add method to write an IpAddr value to a sockaddr.
Posted by GitBox <gi...@apache.org>.
SolidWallOfCode removed a comment on pull request #7821:
URL: https://github.com/apache/trafficserver/pull/7821#issuecomment-840095615
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] SolidWallOfCode merged pull request #7821: Add method to write an IpAddr value to a sockaddr.
Posted by GitBox <gi...@apache.org>.
SolidWallOfCode merged pull request #7821:
URL: https://github.com/apache/trafficserver/pull/7821
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bneradt commented on a change in pull request #7821: Add method to write an IpAddr value to a sockaddr.
Posted by GitBox <gi...@apache.org>.
bneradt commented on a change in pull request #7821:
URL: https://github.com/apache/trafficserver/pull/7821#discussion_r631407924
##########
File path: include/tscore/ink_inet.h
##########
@@ -1231,6 +1231,8 @@ struct IpAddr {
size_t len ///< [in] Size of buffer.
) const;
+ sockaddr *toSockAddr(sockaddr *dest) const;
Review comment:
Probably good to add a doxygen for this to be consistent with the other functions here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] SolidWallOfCode commented on a change in pull request #7821: Add method to write an IpAddr value to a sockaddr.
Posted by GitBox <gi...@apache.org>.
SolidWallOfCode commented on a change in pull request #7821:
URL: https://github.com/apache/trafficserver/pull/7821#discussion_r633692854
##########
File path: src/tscore/ink_inet.cc
##########
@@ -463,6 +463,27 @@ IpAddr::toString(char *dest, size_t len) const
return dest;
}
+sockaddr *
+IpAddr::toSockAddr(sockaddr *dest) const
+{
+ if (AF_INET == _family) {
Review comment:
It's only two values and those values are not compact, not sure a `switch` makes it any better.
##########
File path: src/tscore/ink_inet.cc
##########
@@ -463,6 +463,27 @@ IpAddr::toString(char *dest, size_t len) const
return dest;
}
+sockaddr *
+IpAddr::toSockAddr(sockaddr *dest) const
+{
+ if (AF_INET == _family) {
+ dest->sa_family = AF_INET;
+ ats_ip4_addr_cast(dest) = _addr._ip4;
+#if HAVE_STRUCT_SOCKADDR_IN_SIN_LEN
+ dest->sin_len = sizeof(sockaddr_in);
+#endif
+ } else if (AF_INET6 == _family) {
+ dest->sa_family = AF_INET6;
+ ats_ip6_addr_cast(dest) = _addr._ip6;
+#if HAVE_STRUCT_SOCKADDR_IN_SIN_LEN
+ dest->sin6_len = sizeof(sockaddr_in6);
+#endif
+ } else {
+ dest->sa_family = AF_UNSPEC;
Review comment:
Depends on whether you want to crash on use of an invalid instance. The common use case will be something like
```
sockaddr_storage sa;
int result = bind(sock_fd, addr.toSockAddr(&sa), options);
```
Currently this will fail on an invalid `addr` and be indicated by `result` having a error value. If instead `nullptr` was returned the invalid `addr` would cause a crash. As a user of this, I'd prefer the former.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] SolidWallOfCode commented on a change in pull request #7821: Add method to write an IpAddr value to a sockaddr.
Posted by GitBox <gi...@apache.org>.
SolidWallOfCode commented on a change in pull request #7821:
URL: https://github.com/apache/trafficserver/pull/7821#discussion_r633692854
##########
File path: src/tscore/ink_inet.cc
##########
@@ -463,6 +463,27 @@ IpAddr::toString(char *dest, size_t len) const
return dest;
}
+sockaddr *
+IpAddr::toSockAddr(sockaddr *dest) const
+{
+ if (AF_INET == _family) {
Review comment:
It's only two values and those values are not compact, not sure a `switch` makes it any better.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] SolidWallOfCode commented on a change in pull request #7821: Add method to write an IpAddr value to a sockaddr.
Posted by GitBox <gi...@apache.org>.
SolidWallOfCode commented on a change in pull request #7821:
URL: https://github.com/apache/trafficserver/pull/7821#discussion_r633695685
##########
File path: src/tscore/ink_inet.cc
##########
@@ -463,6 +463,27 @@ IpAddr::toString(char *dest, size_t len) const
return dest;
}
+sockaddr *
+IpAddr::toSockAddr(sockaddr *dest) const
+{
+ if (AF_INET == _family) {
+ dest->sa_family = AF_INET;
+ ats_ip4_addr_cast(dest) = _addr._ip4;
+#if HAVE_STRUCT_SOCKADDR_IN_SIN_LEN
+ dest->sin_len = sizeof(sockaddr_in);
+#endif
+ } else if (AF_INET6 == _family) {
+ dest->sa_family = AF_INET6;
+ ats_ip6_addr_cast(dest) = _addr._ip6;
+#if HAVE_STRUCT_SOCKADDR_IN_SIN_LEN
+ dest->sin6_len = sizeof(sockaddr_in6);
+#endif
+ } else {
+ dest->sa_family = AF_UNSPEC;
Review comment:
Depends on whether you want to crash on use of an invalid instance. The common use case will be something like
```
sockaddr_storage sa;
int result = bind(sock_fd, addr.toSockAddr(&sa), options);
```
Currently this will fail on an invalid `addr` and be indicated by `result` having a error value. If instead `nullptr` was returned the invalid `addr` would cause a crash. As a user of this, I'd prefer the former.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] SolidWallOfCode commented on pull request #7821: Add method to write an IpAddr value to a sockaddr.
Posted by GitBox <gi...@apache.org>.
SolidWallOfCode commented on pull request #7821:
URL: https://github.com/apache/trafficserver/pull/7821#issuecomment-840095880
[approve ci FreeBSD]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] cukiernik commented on a change in pull request #7821: Add method to write an IpAddr value to a sockaddr.
Posted by GitBox <gi...@apache.org>.
cukiernik commented on a change in pull request #7821:
URL: https://github.com/apache/trafficserver/pull/7821#discussion_r632809727
##########
File path: src/tscore/ink_inet.cc
##########
@@ -463,6 +463,27 @@ IpAddr::toString(char *dest, size_t len) const
return dest;
}
+sockaddr *
+IpAddr::toSockAddr(sockaddr *dest) const
+{
+ if (AF_INET == _family) {
+ dest->sa_family = AF_INET;
+ ats_ip4_addr_cast(dest) = _addr._ip4;
+#if HAVE_STRUCT_SOCKADDR_IN_SIN_LEN
+ dest->sin_len = sizeof(sockaddr_in);
+#endif
+ } else if (AF_INET6 == _family) {
+ dest->sa_family = AF_INET6;
+ ats_ip6_addr_cast(dest) = _addr._ip6;
+#if HAVE_STRUCT_SOCKADDR_IN_SIN_LEN
+ dest->sin6_len = sizeof(sockaddr_in6);
+#endif
+ } else {
+ dest->sa_family = AF_UNSPEC;
Review comment:
maybe return nullptr, as fail indicator?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] SolidWallOfCode commented on a change in pull request #7821: Add method to write an IpAddr value to a sockaddr.
Posted by GitBox <gi...@apache.org>.
SolidWallOfCode commented on a change in pull request #7821:
URL: https://github.com/apache/trafficserver/pull/7821#discussion_r631483125
##########
File path: include/tscore/ink_inet.h
##########
@@ -1231,6 +1231,8 @@ struct IpAddr {
size_t len ///< [in] Size of buffer.
) const;
+ sockaddr *toSockAddr(sockaddr *dest) const;
Review comment:
Gah, yeah. Let me fix that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] SolidWallOfCode commented on pull request #7821: Add method to write an IpAddr value to a sockaddr.
Posted by GitBox <gi...@apache.org>.
SolidWallOfCode commented on pull request #7821:
URL: https://github.com/apache/trafficserver/pull/7821#issuecomment-840095615
[ci approve FreeBSD]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] cukiernik commented on a change in pull request #7821: Add method to write an IpAddr value to a sockaddr.
Posted by GitBox <gi...@apache.org>.
cukiernik commented on a change in pull request #7821:
URL: https://github.com/apache/trafficserver/pull/7821#discussion_r632807111
##########
File path: src/tscore/ink_inet.cc
##########
@@ -463,6 +463,27 @@ IpAddr::toString(char *dest, size_t len) const
return dest;
}
+sockaddr *
+IpAddr::toSockAddr(sockaddr *dest) const
+{
+ if (AF_INET == _family) {
Review comment:
maybe use a **switch(_family)**?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] cukiernik commented on a change in pull request #7821: Add method to write an IpAddr value to a sockaddr.
Posted by GitBox <gi...@apache.org>.
cukiernik commented on a change in pull request #7821:
URL: https://github.com/apache/trafficserver/pull/7821#discussion_r632809727
##########
File path: src/tscore/ink_inet.cc
##########
@@ -463,6 +463,27 @@ IpAddr::toString(char *dest, size_t len) const
return dest;
}
+sockaddr *
+IpAddr::toSockAddr(sockaddr *dest) const
+{
+ if (AF_INET == _family) {
+ dest->sa_family = AF_INET;
+ ats_ip4_addr_cast(dest) = _addr._ip4;
+#if HAVE_STRUCT_SOCKADDR_IN_SIN_LEN
+ dest->sin_len = sizeof(sockaddr_in);
+#endif
+ } else if (AF_INET6 == _family) {
+ dest->sa_family = AF_INET6;
+ ats_ip6_addr_cast(dest) = _addr._ip6;
+#if HAVE_STRUCT_SOCKADDR_IN_SIN_LEN
+ dest->sin6_len = sizeof(sockaddr_in6);
+#endif
+ } else {
+ dest->sa_family = AF_UNSPEC;
Review comment:
maybe return nullptr, as a fail indicator?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org