[GitHub] [trafficserver] moonchen commented on a diff in pull request #9825: Clean up UnixNetProcessor entanglements.

["moonchen (via GitHub)" <gi...@apache.org>]

moonchen commented on code in PR #9825:
URL: https://github.com/apache/trafficserver/pull/9825#discussion_r1230179582


##########
iocore/net/UnixNetProcessor.cc:
##########
@@ -259,19 +260,11 @@ UnixNetProcessor::connect_re_internal(Continuation *cont, sockaddr const *target
   }
 }
 
-Action *
-UnixNetProcessor::connect(Continuation *cont, UnixNetVConnection ** /* avc */, sockaddr const *target, NetVCOptions *opt)
-{
-  return connect_re(cont, target, opt);
-}
-
-struct PollCont;
-
 // This needs to be called before the ET_NET threads are started.
 void
 UnixNetProcessor::init()
 {
-  EventType etype = ET_NET;
+  naVecMutex = new_ProxyMutex();

Review Comment:
   It's not intuitive to me that the NetProcessor should be responsible for initializing this mutex, which is defined in UnixNetAccept.cc.  I recommending leaving the initialization in `main` as before.
   
   Alternatively, if you move `naVec` and `naVecMutex` inside of `UnixNetProcessor`, then this would be a good place to initialize `naVecMutex`.



##########
iocore/net/UnixNetProcessor.cc:
##########
@@ -30,7 +30,19 @@
 // For Stat Pages
 #include "StatPages.h"
 
-int net_accept_number = 0;
+static int net_accept_number     = 0;

Review Comment:
   It's not a big deal, but I think a good place for these variables is as static variables inside the functions that they're used.  The reason is that they're only used in those functions, and not meant to be touched anywhere else.



##########
iocore/net/UnixNetProcessor.cc:
##########
@@ -56,34 +68,21 @@ NetProcessor::AcceptOptions::reset()
   return *this;
 }
 
-int net_connection_number = 1;
-
-unsigned int
-net_next_connection_number()
-{
-  unsigned int res = 0;
-  do {
-    res = static_cast<unsigned int>(ink_atomic_increment(&net_connection_number, 1));
-  } while (!res);
-  return res;
-}
-
 Action *
-NetProcessor::accept(Continuation *cont, AcceptOptions const &opt)
+UnixNetProcessor::accept(Continuation *cont, AcceptOptions const &opt)
 {
   Debug("iocore_net_processor", "NetProcessor::accept - port %d,recv_bufsize %d, send_bufsize %d, sockopt 0x%0x", opt.local_port,
         opt.recv_bufsize, opt.send_bufsize, opt.sockopt_flags);
 
-  return ((UnixNetProcessor *)this)->accept_internal(cont, NO_FD, opt);
+  return this->accept_internal(cont, NO_FD, opt);
 }
 
 Action *
-NetProcessor::main_accept(Continuation *cont, SOCKET fd, AcceptOptions const &opt)
+UnixNetProcessor::main_accept(Continuation *cont, SOCKET fd, AcceptOptions const &opt)
 {
-  UnixNetProcessor *this_unp = static_cast<UnixNetProcessor *>(this);
   Debug("iocore_net_processor", "NetProcessor::main_accept - port %d,recv_bufsize %d, send_bufsize %d, sockopt 0x%0x",
         opt.local_port, opt.recv_bufsize, opt.send_bufsize, opt.sockopt_flags);
-  return this_unp->accept_internal(cont, fd, opt);
+  return this->accept_internal(cont, fd, opt);

Review Comment:
   ditto



##########
iocore/net/UnixNetProcessor.cc:
##########
@@ -56,34 +68,21 @@ NetProcessor::AcceptOptions::reset()
   return *this;
 }
 
-int net_connection_number = 1;
-
-unsigned int
-net_next_connection_number()
-{
-  unsigned int res = 0;
-  do {
-    res = static_cast<unsigned int>(ink_atomic_increment(&net_connection_number, 1));
-  } while (!res);
-  return res;
-}
-
 Action *
-NetProcessor::accept(Continuation *cont, AcceptOptions const &opt)
+UnixNetProcessor::accept(Continuation *cont, AcceptOptions const &opt)
 {
   Debug("iocore_net_processor", "NetProcessor::accept - port %d,recv_bufsize %d, send_bufsize %d, sockopt 0x%0x", opt.local_port,
         opt.recv_bufsize, opt.send_bufsize, opt.sockopt_flags);
 
-  return ((UnixNetProcessor *)this)->accept_internal(cont, NO_FD, opt);
+  return this->accept_internal(cont, NO_FD, opt);

Review Comment:
   `this->` is unnecessary.



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