[GitHub] trafficserver pull request #1534: Use StringView for protocol stack to avoid...

[SolidWallOfCode]

Github user SolidWallOfCode closed the pull request at:

https://github.com/apache/trafficserver/pull/1534


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1534: Use StringView for protocol stack to avoid...

[SolidWallOfCode]

Github user SolidWallOfCode commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/1534#discussion_r105075862
  
--- Diff: iocore/net/P_UnixNetVConnection.h ---
@@ -149,21 +149,21 @@ class UnixNetVConnection : public NetVConnection
   // called when handing an  event from this NetVConnection,//
   // or the NetVConnection creation callback.   //
   
-  virtual void set_active_timeout(ink_hrtime timeout_in);
-  virtual void set_inactivity_timeout(ink_hrtime timeout_in);
-  virtual void cancel_active_timeout();
-  virtual void cancel_inactivity_timeout();
-  virtual void set_action(Continuation *c);
-  virtual void add_to_keep_alive_queue();
-  virtual void remove_from_keep_alive_queue();
-  virtual bool add_to_active_queue();
+  void set_active_timeout(ink_hrtime timeout_in) override;
+  void set_inactivity_timeout(ink_hrtime timeout_in) override;
+  void cancel_active_timeout() override;
+  void cancel_inactivity_timeout() override;
+  void set_action(Continuation *c) override;
+  void add_to_keep_alive_queue() override;
+  void remove_from_keep_alive_queue() override;
+  bool add_to_active_queue() override;
   virtual void remove_from_active_queue();
--- End diff --

@bryancall Note `remove_from_active_queue` does not override anything, in 
contrast to the rest of this group of methods.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1534: Use StringView for protocol stack to avoid...

[SolidWallOfCode]

Github user SolidWallOfCode commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/1534#discussion_r105075732
  
--- Diff: proxy/http/HttpSM.cc ---
@@ -8057,14 +8059,13 @@ HttpSM::is_redirect_required()
 
 // Fill in the client protocols used.  Return the number of entries 
returned
 int
-HttpSM::populate_client_protocol(const char **result, int n) const
+HttpSM::populate_client_protocol(ts::StringView *result, int n) const
--- End diff --

This should be fixed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1534: Use StringView for protocol stack to avoid...

[maskit]

Github user maskit commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/1534#discussion_r104723567
  
--- Diff: proxy/http/HttpTransactHeaders.cc ---
@@ -685,6 +687,23 @@ 
HttpTransactHeaders::insert_server_header_in_response(const char *server_tag, in
   }
 }
 
+/// Look up the protocol stack and write it to the @a via_string.
+size_t
+Write_Client_Protocol_Stack(HttpTransact::State *s, char *via_string, 
size_t len)
--- End diff --

write_client_protocol_stack? Is this intentional?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1534: Use StringView for protocol stack to avoid...

[maskit]

Github user maskit commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/1534#discussion_r104721541
  
--- Diff: proxy/http/HttpSM.cc ---
@@ -8057,14 +8059,13 @@ HttpSM::is_redirect_required()
 
 // Fill in the client protocols used.  Return the number of entries 
returned
 int
-HttpSM::populate_client_protocol(const char **result, int n) const
+HttpSM::populate_client_protocol(ts::StringView *result, int n) const
--- End diff --

StringView (w/o namespace) ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1534: Use StringView for protocol stack to avoid...

[SolidWallOfCode]

GitHub user SolidWallOfCode opened a pull request:

https://github.com/apache/trafficserver/pull/1534

Use StringView for protocol stack to avoid calling strlen on strings with a 
known length.

This is an alternative proposal to #1520.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/SolidWallOfCode/trafficserver pr-1520

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/trafficserver/pull/1534.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1534


commit 37e9e120bd47ef9a14abf4b221871fc1b8d26658
Author: Kit Chan 
Date:   2017-03-01T07:27:37Z

report protocol in request via header

commit dbd191fcac28f6faa9f6465f4b62ab868bd59989
Author: Alan M. Carroll 
Date:   2017-03-04T16:46:33Z

PR #1520 rework.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---