You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "SolidWallOfCode (via GitHub)" <gi...@apache.org> on 2023/12/18 18:35:39 UTC
[PR] Update to libswoc 1.5.9 [trafficserver]
SolidWallOfCode opened a new pull request, #10944:
URL: https://github.com/apache/trafficserver/pull/10944
This is in preparation for some significant ATS updates, including
* Avoiding `ats_ip_copy`
* Final removal of the old `Errata`
* Replacement of `Arena` by `MemArena`.
Release notes:
Just in time for the holidays, a new release of libSWOC! This is a bit bigger than other recent point releases due to increased integration in ATS.
* Fixed possible overflow bug in `svtoi`.
* IP networking support
* Generalization of network category (multicast, private, etc.).
* Better endian handling.
* Added some missing / unimplemented methods.
* Errata: Auto text support for error codes in constructor and `note` method.
* MemArena - support `discard` allocated spans.
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update to libswoc 1.5.9 [trafficserver]
Posted by "ywkaras (via GitHub)" <gi...@apache.org>.
ywkaras commented on code in PR #10944:
URL: https://github.com/apache/trafficserver/pull/10944#discussion_r1430881724
##########
lib/swoc/include/swoc/IPSrv.h:
##########
@@ -289,88 +291,91 @@ class IPSrv {
*/
self_type &assign(IP6Addr const &addr);
- /** Change the address.
+ /** Assign an address.
*
- * @param addr Address to assign.
+ * @param addr Address.
* @return @a this
*
- * If @a addr isn't valid then no assignment is made.
+ * If @a addr isn't valid then no assignment is made, otherwise the family is changed to that of
+ * @a addr.
*/
self_type &assign(IPAddr const &addr);
- /** Change the host_order_port.
+ /** Assign port.
*
- * @param port Port in host order.
+ * @param port Port [host order].
* @return @a this.
*/
self_type &assign(in_port_t port);
- /** Change the address and port.
+ /** Assign an IPv4 address and port.
*
- * @param addr Address to assign.
- * @param port Port to assign.
+ * @param addr Address.
+ * @param port Port [host order].
* @return @a this
*/
self_type &assign(IP4Addr const &addr, in_port_t port);
- /** Change the address and port.
+ /** Assign an IPv6 address and port.
*
- * @param addr Address to assign.
- * @param port Port to assign.
+ * @param addr Address.
+ * @param port Port [host order].
* @return @a this
*/
self_type &assign(IP6Addr const &addr, in_port_t port);
- /** Change the address and port.
+ /** Assogm address amd [prt/
Review Comment:
Wha?
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update to libswoc 1.5.9 [trafficserver]
Posted by "ywkaras (via GitHub)" <gi...@apache.org>.
ywkaras commented on code in PR #10944:
URL: https://github.com/apache/trafficserver/pull/10944#discussion_r1430905082
##########
lib/swoc/include/swoc/swoc_ip_util.h:
##########
@@ -0,0 +1,148 @@
+// SPDX-License-Identifier: Apache-2.0
+// Copyright Network Geographics 2014
+/** @file
+ Shared utilities for IP address classes.
+ */
+
+#pragma once
+#include <netinet/in.h>
+
+#include "swoc/swoc_version.h"
+
+// These have to be global namespace, unfortunately.
+inline bool
+operator==(in6_addr const& lhs, in6_addr const& rhs) {
+ return 0 == memcmp(&lhs, &rhs, sizeof(in6_addr));
+}
+
+inline bool
+operator!=(in6_addr const& lhs, in6_addr const& rhs) {
+ return 0 != memcmp(&lhs, &rhs, sizeof(in6_addr));
+}
+
+namespace swoc { inline namespace SWOC_VERSION_NS {
+
+/// Internal IP address utilities.
+namespace ip {
+
+inline bool is_loopback_host_order(in_addr_t addr) {
+ return (addr & 0xFF000000) == 0x7F000000;
+}
+
+inline bool is_link_local_host_order(in_addr_t addr) {
+ return (addr & 0xFFFF0000) == 0xA9FE0000; // 169.254.0.0/16
Review Comment:
`((169 << 24)|(254 << 16))` is less error-prone.
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update to libswoc 1.5.9 [trafficserver]
Posted by "ywkaras (via GitHub)" <gi...@apache.org>.
ywkaras commented on code in PR #10944:
URL: https://github.com/apache/trafficserver/pull/10944#discussion_r1430879054
##########
lib/swoc/include/swoc/IPSrv.h:
##########
@@ -116,7 +116,7 @@ class IP4Srv {
protected:
IP4Addr _addr; ///< Address.
- in_port_t _port = 0; ///< Port.
+ in_port_t _port = 0; ///< Port [host order].
Review Comment:
why [ host order] not (host order)?
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update to libswoc 1.5.9 [trafficserver]
Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on PR #10944:
URL: https://github.com/apache/trafficserver/pull/10944#issuecomment-1875643260
> We don't make local changes for libswoc on ATS repo, right? If 1.5.9 has issues we simply don't use the version, and hope a new version doesn't have issues. Also, code change for libsowc should be reviewed on its repo but not here.
Correct. I'm going to go ahead and merge this in before things conflict with it.
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update to libswoc 1.5.9 [trafficserver]
Posted by "ywkaras (via GitHub)" <gi...@apache.org>.
ywkaras commented on code in PR #10944:
URL: https://github.com/apache/trafficserver/pull/10944#discussion_r1430763724
##########
lib/swoc/include/swoc/Errata.h:
##########
@@ -337,26 +337,38 @@ class Errata {
* @param text Text of the message.
* @return *this
*
- * The error code is set to the default.
* @a text is localized to @a this and does not need to be persistent.
* The severity is updated to @a severity if the latter is more severe.
*/
self_type ¬e(Severity severity, std::string_view text);
+ /** Add an @c Annotation to the top with @a text and local @a severity.
Review Comment:
text? cut-n-paste error?
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update to libswoc 1.5.9 [trafficserver]
Posted by "ywkaras (via GitHub)" <gi...@apache.org>.
ywkaras commented on code in PR #10944:
URL: https://github.com/apache/trafficserver/pull/10944#discussion_r1430872877
##########
lib/swoc/include/swoc/IPAddr.h:
##########
@@ -1388,14 +1383,12 @@ inline IPAddr::operator IP6Addr() const {
inline bool
IPAddr::operator==(self_type const &that) const {
Review Comment:
self_type has more characters than IPAddr. Are you having a hard time staying busy in retirement?
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update to libswoc 1.5.9 [trafficserver]
Posted by "ywkaras (via GitHub)" <gi...@apache.org>.
ywkaras commented on code in PR #10944:
URL: https://github.com/apache/trafficserver/pull/10944#discussion_r1430914857
##########
lib/swoc/include/swoc/swoc_ip_util.h:
##########
@@ -0,0 +1,148 @@
+// SPDX-License-Identifier: Apache-2.0
+// Copyright Network Geographics 2014
+/** @file
+ Shared utilities for IP address classes.
+ */
+
+#pragma once
+#include <netinet/in.h>
+
+#include "swoc/swoc_version.h"
+
+// These have to be global namespace, unfortunately.
+inline bool
+operator==(in6_addr const& lhs, in6_addr const& rhs) {
+ return 0 == memcmp(&lhs, &rhs, sizeof(in6_addr));
+}
+
+inline bool
+operator!=(in6_addr const& lhs, in6_addr const& rhs) {
+ return 0 != memcmp(&lhs, &rhs, sizeof(in6_addr));
+}
+
+namespace swoc { inline namespace SWOC_VERSION_NS {
+
+/// Internal IP address utilities.
+namespace ip {
+
+inline bool is_loopback_host_order(in_addr_t addr) {
+ return (addr & 0xFF000000) == 0x7F000000;
+}
+
+inline bool is_link_local_host_order(in_addr_t addr) {
+ return (addr & 0xFFFF0000) == 0xA9FE0000; // 169.254.0.0/16
+}
+
+inline bool is_multicast_host_order(in_addr_t addr) {
+ return IN_MULTICAST(addr);
+}
+
+inline bool is_private_host_order(in_addr_t addr) {
+ return (((addr & 0xFF000000) == 0x0A000000) || // 10.0.0.0/8
+ ((addr & 0xFFC00000) == 0x64400000) || // 100.64.0.0/10
+ ((addr & 0xFFF00000) == 0xAC100000) || // 172.16.0.0/12
+ ((addr & 0xFFFF0000) == 0xC0A80000) // 192.168.0.0/16
+ );
+}
+
+// There really is no "host order" for IPv6, so only the network order utilities are defined.
+// @c IP6Addr uses an idiosyncratic ordering for performance, not really useful to expose to
+// clients.
+
+inline bool is_loopback_network_order(in6_addr const& addr) {
+ return addr == in6addr_loopback;
+}
+
+inline bool is_multicast_network_order(in6_addr const& addr) {
+ return addr.s6_addr[0] == 0xFF;
+}
+
+inline bool is_link_local_network_order(in6_addr const& addr) {
+ return addr.s6_addr[0] == 0xFE && (addr.s6_addr[1] & 0xC0) == 0x80; // fe80::/10
+}
+
+inline bool is_private_network_order(in6_addr const& addr) {
+ return (addr.s6_addr[0] & 0xFE) == 0xFC; // fc00::/7
+}
+
+#if BYTE_ORDER == LITTLE_ENDIAN
Review Comment:
I think it would be more robust to use:
`#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__`
https://gcc.gnu.org/onlinedocs/cpp/Common-Predefined-Macros.html
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update to libswoc 1.5.9 [trafficserver]
Posted by "ywkaras (via GitHub)" <gi...@apache.org>.
ywkaras commented on code in PR #10944:
URL: https://github.com/apache/trafficserver/pull/10944#discussion_r1430899048
##########
lib/swoc/include/swoc/MemArena.h:
##########
@@ -421,11 +482,10 @@ class MemArena
static constexpr size_t DEFAULT_BLOCK_SIZE = Page::SCALE - Paragraph{round_up(ALLOC_HEADER_SIZE + sizeof(Block))};
size_t _active_allocated = 0; ///< Total allocations in the active generation.
- size_t _active_reserved = 0; ///< Total current reserved memory.
+ size_t _active_reserved = 0; ///< Total reserved memory (allocated from OS).
Review Comment:
I think you mean "allocated from the compiler runtime library allocator"?
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update to libswoc 1.5.9 [trafficserver]
Posted by "ywkaras (via GitHub)" <gi...@apache.org>.
ywkaras commented on code in PR #10944:
URL: https://github.com/apache/trafficserver/pull/10944#discussion_r1430901280
##########
lib/swoc/include/swoc/TextView.h:
##########
@@ -1057,15 +1057,15 @@ uintmax_t svtou(TextView src, TextView *parsed = nullptr, int base = 0);
template <int RADIX>
uintmax_t
svto_radix(TextView &src) {
- static_assert(0 < RADIX && RADIX <= 36, "Radix must be in the range 1..36");
+ static_assert(1 <= RADIX && RADIX <= 36, "Radix must be in the range 2..36");
Review Comment:
should be 1 < RADIX
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update to libswoc 1.5.9 [trafficserver]
Posted by "ywkaras (via GitHub)" <gi...@apache.org>.
ywkaras commented on code in PR #10944:
URL: https://github.com/apache/trafficserver/pull/10944#discussion_r1430932466
##########
lib/swoc/src/TextView.cc:
##########
@@ -103,6 +104,9 @@ svtou(TextView src, TextView *out, int base) {
}
}
}
+ if (!(1 <= base && base <= 36)) {
Review Comment:
How can you have a base 1 number representation?
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update to libswoc 1.5.9 [trafficserver]
Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit commented on PR #10944:
URL: https://github.com/apache/trafficserver/pull/10944#issuecomment-1863683938
We don't make local changes for libswoc on ATS repo, right? If 1.5.9 has issues we simply don't use the version, and hope a new version doesn't have issues. Also, code change for libsowc should be reviewed on its repo but not 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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update to libswoc 1.5.9 [trafficserver]
Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt merged PR #10944:
URL: https://github.com/apache/trafficserver/pull/10944
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org