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/03/17 00:55:01 UTC
[GitHub] [trafficserver] SolidWallOfCode opened a new pull request, #9531: Update logfilter to remove use of IpMap
SolidWallOfCode opened a new pull request, #9531:
URL: https://github.com/apache/trafficserver/pull/9531
Requires #9530 - libswoc 1.4.6.
--
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
[GitHub] [trafficserver] SolidWallOfCode commented on a diff in pull request #9531: Update logfilter to remove use of IpMap
Posted by "SolidWallOfCode (via GitHub)" <gi...@apache.org>.
SolidWallOfCode commented on code in PR #9531:
URL: https://github.com/apache/trafficserver/pull/9531#discussion_r1140299632
##########
lib/swoc/include/swoc/IPRange.h:
##########
@@ -1145,22 +1206,85 @@ class IPRangeSet
/// @return Number of ranges in the set.
size_t count() const;
+ bool empty() const;
+
/// Remove all addresses in the set.
void clear();
-protected:
- /// Empty struct to use for payload.
- /// This declares the struct and defines the singleton instance used.
- static inline constexpr struct Mark {
- using self_type = Mark;
- /// @internal @c IPSpace requires equality / inequality operators.
- /// These make all instance equal to each other.
- bool operator==(self_type const &that);
- bool operator!=(self_type const &that);
- } MARK{};
+ /// Constant iterator for iteration over ranges.
+ class const_iterator {
Review Comment:
My understanding is that inheriting from iterator tags is deprecated and instead the types should be provided via `using`.
--
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
[GitHub] [trafficserver] SolidWallOfCode merged pull request #9531: Update logfilter to remove use of IpMap
Posted by "SolidWallOfCode (via GitHub)" <gi...@apache.org>.
SolidWallOfCode merged PR #9531:
URL: https://github.com/apache/trafficserver/pull/9531
--
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
[GitHub] [trafficserver] SolidWallOfCode commented on pull request #9531: Update logfilter to remove use of IpMap
Posted by "SolidWallOfCode (via GitHub)" <gi...@apache.org>.
SolidWallOfCode commented on PR #9531:
URL: https://github.com/apache/trafficserver/pull/9531#issuecomment-1473920778
Yes, I was getting failures with "log-filter" (among others) which I fixed before sending up this PR.
--
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
[GitHub] [trafficserver] ywkaras commented on a diff in pull request #9531: Update logfilter to remove use of IpMap
Posted by "ywkaras (via GitHub)" <gi...@apache.org>.
ywkaras commented on code in PR #9531:
URL: https://github.com/apache/trafficserver/pull/9531#discussion_r1141030832
##########
lib/swoc/include/swoc/IPRange.h:
##########
@@ -1145,22 +1206,85 @@ class IPRangeSet
/// @return Number of ranges in the set.
size_t count() const;
+ bool empty() const;
+
/// Remove all addresses in the set.
void clear();
-protected:
- /// Empty struct to use for payload.
- /// This declares the struct and defines the singleton instance used.
- static inline constexpr struct Mark {
- using self_type = Mark;
- /// @internal @c IPSpace requires equality / inequality operators.
- /// These make all instance equal to each other.
- bool operator==(self_type const &that);
- bool operator!=(self_type const &that);
- } MARK{};
+ /// Constant iterator for iteration over ranges.
+ class const_iterator {
Review Comment:
So we're manually copying libswoc code and checking it into TS? I wonder if we could implement this logic in the make file:
```
LIBSWOC_VERSION=yada
if [[ ! -d libswoc || ! -f libswoc/version || $LIBSWOC != $(cat libswoc/version) ]] ; then
rm -rf libswoc
git clone something-something libswoc
echo $LIBSWOC_VERSION > libswoc/version # even better if libswoc alread has this file
fi
```
and also be non-Damian and update .gitignore to ignore these source files.
--
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
[GitHub] [trafficserver] ywkaras commented on a diff in pull request #9531: Update logfilter to remove use of IpMap
Posted by "ywkaras (via GitHub)" <gi...@apache.org>.
ywkaras commented on code in PR #9531:
URL: https://github.com/apache/trafficserver/pull/9531#discussion_r1139522669
##########
lib/swoc/include/swoc/IPRange.h:
##########
@@ -1145,22 +1206,85 @@ class IPRangeSet
/// @return Number of ranges in the set.
size_t count() const;
+ bool empty() const;
+
/// Remove all addresses in the set.
void clear();
-protected:
- /// Empty struct to use for payload.
- /// This declares the struct and defines the singleton instance used.
- static inline constexpr struct Mark {
- using self_type = Mark;
- /// @internal @c IPSpace requires equality / inequality operators.
- /// These make all instance equal to each other.
- bool operator==(self_type const &that);
- bool operator!=(self_type const &that);
- } MARK{};
+ /// Constant iterator for iteration over ranges.
+ class const_iterator {
Review Comment:
Say what type or the concepts: https://en.cppreference.com/w/cpp/iterator
--
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
[GitHub] [trafficserver] ywkaras commented on a diff in pull request #9531: Update logfilter to remove use of IpMap
Posted by "ywkaras (via GitHub)" <gi...@apache.org>.
ywkaras commented on code in PR #9531:
URL: https://github.com/apache/trafficserver/pull/9531#discussion_r1140382500
##########
lib/swoc/include/swoc/IPRange.h:
##########
@@ -1145,22 +1206,85 @@ class IPRangeSet
/// @return Number of ranges in the set.
size_t count() const;
+ bool empty() const;
+
/// Remove all addresses in the set.
void clear();
-protected:
- /// Empty struct to use for payload.
- /// This declares the struct and defines the singleton instance used.
- static inline constexpr struct Mark {
- using self_type = Mark;
- /// @internal @c IPSpace requires equality / inequality operators.
- /// These make all instance equal to each other.
- bool operator==(self_type const &that);
- bool operator!=(self_type const &that);
- } MARK{};
+ /// Constant iterator for iteration over ranges.
+ class const_iterator {
Review Comment:
I don't understand your response. You're adding a new iterator type. Seems like a good idea to include a comment that it's in the LegacyBidirectionalIterator category or whatever. If it's not STL compatible, probably better to not use a name that implies it's STL compatible.
--
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
[GitHub] [trafficserver] SolidWallOfCode commented on a diff in pull request #9531: Update logfilter to remove use of IpMap
Posted by "SolidWallOfCode (via GitHub)" <gi...@apache.org>.
SolidWallOfCode commented on code in PR #9531:
URL: https://github.com/apache/trafficserver/pull/9531#discussion_r1140299632
##########
lib/swoc/include/swoc/IPRange.h:
##########
@@ -1145,22 +1206,85 @@ class IPRangeSet
/// @return Number of ranges in the set.
size_t count() const;
+ bool empty() const;
+
/// Remove all addresses in the set.
void clear();
-protected:
- /// Empty struct to use for payload.
- /// This declares the struct and defines the singleton instance used.
- static inline constexpr struct Mark {
- using self_type = Mark;
- /// @internal @c IPSpace requires equality / inequality operators.
- /// These make all instance equal to each other.
- bool operator==(self_type const &that);
- bool operator!=(self_type const &that);
- } MARK{};
+ /// Constant iterator for iteration over ranges.
+ class const_iterator {
Review Comment:
My understanding is that inheriting from iterator tags is deprecated and instead the types should be provided via `using`.
Also, that can't be fixed in this PR, you'd need to submit a PR to libswoc.
--
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
[GitHub] [trafficserver] SolidWallOfCode commented on a diff in pull request #9531: Update logfilter to remove use of IpMap
Posted by "SolidWallOfCode (via GitHub)" <gi...@apache.org>.
SolidWallOfCode commented on code in PR #9531:
URL: https://github.com/apache/trafficserver/pull/9531#discussion_r1141026052
##########
lib/swoc/include/swoc/IPRange.h:
##########
@@ -1145,22 +1206,85 @@ class IPRangeSet
/// @return Number of ranges in the set.
size_t count() const;
+ bool empty() const;
+
/// Remove all addresses in the set.
void clear();
-protected:
- /// Empty struct to use for payload.
- /// This declares the struct and defines the singleton instance used.
- static inline constexpr struct Mark {
- using self_type = Mark;
- /// @internal @c IPSpace requires equality / inequality operators.
- /// These make all instance equal to each other.
- bool operator==(self_type const &that);
- bool operator!=(self_type const &that);
- } MARK{};
+ /// Constant iterator for iteration over ranges.
+ class const_iterator {
Review Comment:
OK, I'll put an update in libswoc.
--
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
[GitHub] [trafficserver] ywkaras commented on a diff in pull request #9531: Update logfilter to remove use of IpMap
Posted by "ywkaras (via GitHub)" <gi...@apache.org>.
ywkaras commented on code in PR #9531:
URL: https://github.com/apache/trafficserver/pull/9531#discussion_r1141030832
##########
lib/swoc/include/swoc/IPRange.h:
##########
@@ -1145,22 +1206,85 @@ class IPRangeSet
/// @return Number of ranges in the set.
size_t count() const;
+ bool empty() const;
+
/// Remove all addresses in the set.
void clear();
-protected:
- /// Empty struct to use for payload.
- /// This declares the struct and defines the singleton instance used.
- static inline constexpr struct Mark {
- using self_type = Mark;
- /// @internal @c IPSpace requires equality / inequality operators.
- /// These make all instance equal to each other.
- bool operator==(self_type const &that);
- bool operator!=(self_type const &that);
- } MARK{};
+ /// Constant iterator for iteration over ranges.
+ class const_iterator {
Review Comment:
So we're manually copying libswoc code and checking it into TS? I wonder if we could implement this logic in the make file:
```
LIBSWOC_VERSION=yada
if [[ ! -d libswoc || ! -f libswoc/version || $LIBSWOC != $(cat libswoc/version) ]] ; then
rm -rf libswoc
git clone something-something libswoc
echo $LIBSWOC_VERSION > libswoc/version # even better if libswoc already has this file
fi
```
and also be non-Damian and update .gitignore to ignore these source files.
--
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
[GitHub] [trafficserver] SolidWallOfCode commented on a diff in pull request #9531: Update logfilter to remove use of IpMap
Posted by "SolidWallOfCode (via GitHub)" <gi...@apache.org>.
SolidWallOfCode commented on code in PR #9531:
URL: https://github.com/apache/trafficserver/pull/9531#discussion_r1141455488
##########
lib/swoc/include/swoc/IPRange.h:
##########
@@ -1145,22 +1206,85 @@ class IPRangeSet
/// @return Number of ranges in the set.
size_t count() const;
+ bool empty() const;
+
/// Remove all addresses in the set.
void clear();
-protected:
- /// Empty struct to use for payload.
- /// This declares the struct and defines the singleton instance used.
- static inline constexpr struct Mark {
- using self_type = Mark;
- /// @internal @c IPSpace requires equality / inequality operators.
- /// These make all instance equal to each other.
- bool operator==(self_type const &that);
- bool operator!=(self_type const &that);
- } MARK{};
+ /// Constant iterator for iteration over ranges.
+ class const_iterator {
Review Comment:
It was decided to do it this way because that's how the YAML library is handled. You can look [here](https://github.com/SolidWallOfCode/libswoc/blob/master/tools/ats-drop.sh) for how the code is copied. That could be move over to ATS and then invoked from a makefile.
--
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
[GitHub] [trafficserver] ywkaras commented on pull request #9531: Update logfilter to remove use of IpMap
Posted by "ywkaras (via GitHub)" <gi...@apache.org>.
ywkaras commented on PR #9531:
URL: https://github.com/apache/trafficserver/pull/9531#issuecomment-1472973835
Are there Au test(s) that exercise this these changes?
--
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