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 2020/05/22 15:26:08 UTC
[GitHub] [trafficserver] brbzull0 opened a new pull request #6820: HostDB - change HostResPreferenceOrder type to std::array.
brbzull0 opened a new pull request #6820:
URL: https://github.com/apache/trafficserver/pull/6820
Using `std::array` instead of c style array.
This change make easy and less verbose dealing with the host resolution type, better value semantics in that sense.
----------------------------------------------------------------
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] brbzull0 commented on pull request #6820: HostDB - change HostResPreferenceOrder type to std::array.
Posted by GitBox <gi...@apache.org>.
brbzull0 commented on pull request #6820:
URL: https://github.com/apache/trafficserver/pull/6820#issuecomment-634622575
[approve ci autest]
----------------------------------------------------------------
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 #6820: HostDB - change HostResPreferenceOrder type to std::array.
Posted by GitBox <gi...@apache.org>.
SolidWallOfCode commented on a change in pull request #6820:
URL: https://github.com/apache/trafficserver/pull/6820#discussion_r430711209
##########
File path: include/tscore/ink_resolver.h
##########
@@ -157,10 +159,8 @@ enum HostResPreference {
};
/// # of preference values.
static int const N_HOST_RES_PREFERENCE = HOST_RES_PREFER_IPV6 + 1;
-/// # of entries in a preference ordering.
-static int const N_HOST_RES_PREFERENCE_ORDER = 3;
/// Storage for preference ordering.
-typedef HostResPreference HostResPreferenceOrder[N_HOST_RES_PREFERENCE_ORDER];
+using HostResPreferenceOrder = std::array<HostResPreference, 3>;
Review comment:
Why not `N_HOST_RES_PREFERENCE` instead of `3`? Avoid explicit numbers where possible. Another alternative is to get rid of `N_HOST_RES_PREFERENCE` and put the computation for that value in place of 3, then use `.size()` as needed.
----------------------------------------------------------------
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] brbzull0 commented on a change in pull request #6820: HostDB - change HostResPreferenceOrder type to std::array.
Posted by GitBox <gi...@apache.org>.
brbzull0 commented on a change in pull request #6820:
URL: https://github.com/apache/trafficserver/pull/6820#discussion_r429358921
##########
File path: src/tscore/ink_res_init.cc
##########
@@ -601,7 +601,8 @@ parse_host_res_preference(const char *value, HostResPreferenceOrder order)
found[i] = false;
}
- for (i = 0; i < n && np < N_HOST_RES_PREFERENCE_ORDER; ++i) {
+ const int order_size = static_cast<int>(order.size());
+ for (i = 0; i < n && np < order_size; ++i) {
Review comment:
I though that first, but the loop seems to rely more on the `token` than the actual `order` array. I haven't seen begin/end on the Tokenizer class either.
----------------------------------------------------------------
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] randall commented on pull request #6820: HostDB - change HostResPreferenceOrder type to std::array.
Posted by GitBox <gi...@apache.org>.
randall commented on pull request #6820:
URL: https://github.com/apache/trafficserver/pull/6820#issuecomment-634189178
----------------------------------------------------------------
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] brbzull0 commented on pull request #6820: HostDB - change HostResPreferenceOrder type to std::array.
Posted by GitBox <gi...@apache.org>.
brbzull0 commented on pull request #6820:
URL: https://github.com/apache/trafficserver/pull/6820#issuecomment-634257511
[approve ci autest]
----------------------------------------------------------------
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] randall merged pull request #6820: HostDB - change HostResPreferenceOrder type to std::array.
Posted by GitBox <gi...@apache.org>.
randall merged pull request #6820:
URL: https://github.com/apache/trafficserver/pull/6820
----------------------------------------------------------------
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] bryancall commented on a change in pull request #6820: HostDB - change HostResPreferenceOrder type to std::array.
Posted by GitBox <gi...@apache.org>.
bryancall commented on a change in pull request #6820:
URL: https://github.com/apache/trafficserver/pull/6820#discussion_r429329262
##########
File path: src/tscore/ink_res_init.cc
##########
@@ -601,7 +601,8 @@ parse_host_res_preference(const char *value, HostResPreferenceOrder order)
found[i] = false;
}
- for (i = 0; i < n && np < N_HOST_RES_PREFERENCE_ORDER; ++i) {
+ const int order_size = static_cast<int>(order.size());
+ for (i = 0; i < n && np < order_size; ++i) {
Review comment:
Can't this be a range-based for loop?
----------------------------------------------------------------
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