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/04/29 16:36:03 UTC

[GitHub] [trafficserver] a-canary opened a new pull request #6721: HttpSM cleaning up non-sense pointer indirection

a-canary opened a new pull request #6721:
URL: https://github.com/apache/trafficserver/pull/6721


   removing pointers that seem to go in circles. I'm assuming this is the result of many copy and pastes.


----------------------------------------------------------------
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] shinrich commented on pull request #6721: HttpSM cleaning up non-sense pointer indirection

Posted by GitBox <gi...@apache.org>.
shinrich commented on pull request #6721:
URL: https://github.com/apache/trafficserver/pull/6721#issuecomment-622533625


   [approve ci clang-analyzer]


----------------------------------------------------------------
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] shinrich commented on a change in pull request #6721: HttpSM cleaning up non-sense pointer indirection

Posted by GitBox <gi...@apache.org>.
shinrich commented on a change in pull request #6721:
URL: https://github.com/apache/trafficserver/pull/6721#discussion_r418569433



##########
File path: proxy/http/HttpSM.cc
##########
@@ -638,19 +638,19 @@ HttpSM::setup_blind_tunnel_port()
         if (ssl_vc->get_tunnel_port() > 0) {
           t_state.hdr_info.client_request.url_get()->port_set(ssl_vc->get_tunnel_port());
         } else {
-          t_state.hdr_info.client_request.url_get()->port_set(t_state.state_machine->ua_txn->get_netvc()->get_local_port());
+          t_state.hdr_info.client_request.url_get()->port_set(netvc->get_local_port());
         }
       } else {
         t_state.hdr_info.client_request.url_get()->host_set(ssl_vc->get_server_name(), strlen(ssl_vc->get_server_name()));
-        t_state.hdr_info.client_request.url_get()->port_set(t_state.state_machine->ua_txn->get_netvc()->get_local_port());
+        t_state.hdr_info.client_request.url_get()->port_set(netvc->get_local_port());
       }
     }
   } else {
     char new_host[INET6_ADDRSTRLEN];
-    ats_ip_ntop(t_state.state_machine->ua_txn->get_netvc()->get_local_addr(), new_host, sizeof(new_host));
+    ats_ip_ntop(netvc->get_local_addr(), new_host, sizeof(new_host));

Review comment:
       However clang analyzer points out that you did't verify that netvc isn't null here.  However, I would think the origin chain would have had the same issue.




----------------------------------------------------------------
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] a-canary commented on a change in pull request #6721: HttpSM cleaning up non-sense pointer indirection

Posted by GitBox <gi...@apache.org>.
a-canary commented on a change in pull request #6721:
URL: https://github.com/apache/trafficserver/pull/6721#discussion_r418591026



##########
File path: proxy/http/HttpSM.cc
##########
@@ -638,19 +638,19 @@ HttpSM::setup_blind_tunnel_port()
         if (ssl_vc->get_tunnel_port() > 0) {
           t_state.hdr_info.client_request.url_get()->port_set(ssl_vc->get_tunnel_port());
         } else {
-          t_state.hdr_info.client_request.url_get()->port_set(t_state.state_machine->ua_txn->get_netvc()->get_local_port());
+          t_state.hdr_info.client_request.url_get()->port_set(netvc->get_local_port());
         }
       } else {
         t_state.hdr_info.client_request.url_get()->host_set(ssl_vc->get_server_name(), strlen(ssl_vc->get_server_name()));
-        t_state.hdr_info.client_request.url_get()->port_set(t_state.state_machine->ua_txn->get_netvc()->get_local_port());
+        t_state.hdr_info.client_request.url_get()->port_set(netvc->get_local_port());
       }
     }
   } else {
     char new_host[INET6_ADDRSTRLEN];
-    ats_ip_ntop(t_state.state_machine->ua_txn->get_netvc()->get_local_addr(), new_host, sizeof(new_host));
+    ats_ip_ntop(netvc->get_local_addr(), new_host, sizeof(new_host));

Review comment:
       I added an `ink_release_assert(netvc);` to give clang a valid code path. 




----------------------------------------------------------------
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