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/05 20:19:17 UTC
[GitHub] [trafficserver] jrushford opened a new pull request #7787: Fixes an issue where next hop unit tests crash when run on macOS.
jrushford opened a new pull request #7787:
URL: https://github.com/apache/trafficserver/pull/7787
Fixes an issue when running the Next Hop unit tests on macOS. macOS crashes due to a SIGABRT when a read/write lock cannot be obtained to read a config variable in the NextHopSelectStrategy constructor. This PR basically creates a mock around REC_ReadConfigInteger().
--
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] jrushford commented on a change in pull request #7787: Fixes an issue where next hop unit tests crash when run on macOS.
Posted by GitBox <gi...@apache.org>.
jrushford commented on a change in pull request #7787:
URL: https://github.com/apache/trafficserver/pull/7787#discussion_r627543979
##########
File path: proxy/http/remap/unit-tests/nexthop_test_stubs.cc
##########
@@ -125,6 +125,14 @@ build_request(int64_t sm_id, HttpSM *sm, sockaddr_in *ip, const char *os_hostnam
sm->t_state.txn_conf = reinterpret_cast<OverridableHttpConfigParams *>(_my_txn_conf);
}
+void
+GetConfigInteger(int *v, const char *n)
+{
+ if (strncmp(n, "proxy.config.http.parent_proxy.max_trans_retries", strlen(n)) == 0) {
Review comment:
@ywkaras I made the change to strcmp() and added some nullptr checks.
--
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] ywkaras commented on a change in pull request #7787: Fixes an issue where next hop unit tests crash when run on macOS.
Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #7787:
URL: https://github.com/apache/trafficserver/pull/7787#discussion_r627433349
##########
File path: proxy/http/remap/unit-tests/nexthop_test_stubs.cc
##########
@@ -125,6 +125,14 @@ build_request(int64_t sm_id, HttpSM *sm, sockaddr_in *ip, const char *os_hostnam
sm->t_state.txn_conf = reinterpret_cast<OverridableHttpConfigParams *>(_my_txn_conf);
}
+void
+GetConfigInteger(int *v, const char *n)
+{
+ if (strncmp(n, "proxy.config.http.parent_proxy.max_trans_retries", strlen(n)) == 0) {
Review comment:
I think the safety issue is that a C string might have a missing null terminator. But strncmp() only helps with that if it's got a constant as the length. strcmp() is a safe as strncmp() if one of the strings is a string literal.
--
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] jrushford merged pull request #7787: Fixes an issue where next hop unit tests crash when run on macOS.
Posted by GitBox <gi...@apache.org>.
jrushford merged pull request #7787:
URL: https://github.com/apache/trafficserver/pull/7787
--
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] jrushford commented on a change in pull request #7787: Fixes an issue where next hop unit tests crash when run on macOS.
Posted by GitBox <gi...@apache.org>.
jrushford commented on a change in pull request #7787:
URL: https://github.com/apache/trafficserver/pull/7787#discussion_r627087827
##########
File path: proxy/http/remap/unit-tests/nexthop_test_stubs.cc
##########
@@ -125,6 +125,14 @@ build_request(int64_t sm_id, HttpSM *sm, sockaddr_in *ip, const char *os_hostnam
sm->t_state.txn_conf = reinterpret_cast<OverridableHttpConfigParams *>(_my_txn_conf);
}
+void
+GetConfigInteger(int *v, const char *n)
+{
+ if (strncmp(n, "proxy.config.http.parent_proxy.max_trans_retries", strlen(n)) == 0) {
Review comment:
@ywkaras I think strncmp() is safer. Why do you prefer strcmp()?
--
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] ywkaras commented on a change in pull request #7787: Fixes an issue where next hop unit tests crash when run on macOS.
Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #7787:
URL: https://github.com/apache/trafficserver/pull/7787#discussion_r626946365
##########
File path: proxy/http/remap/unit-tests/nexthop_test_stubs.cc
##########
@@ -125,6 +125,14 @@ build_request(int64_t sm_id, HttpSM *sm, sockaddr_in *ip, const char *os_hostnam
sm->t_state.txn_conf = reinterpret_cast<OverridableHttpConfigParams *>(_my_txn_conf);
}
+void
+GetConfigInteger(int *v, const char *n)
+{
+ if (strncmp(n, "proxy.config.http.parent_proxy.max_trans_retries", strlen(n)) == 0) {
Review comment:
Why not just use strcmp()?
--
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