issue: 3788369 Remove object pointer from the list_node

list_node is always inlined to a class and its offset is known.
Therefore, we can find the object pointer with a single arithmetic
operation and save 8 bytes per list_node.

Remove the list iterator. It's mostly unused and requires special
treatment of the head and tail. Replace it with next() and prev().

Signed-off-by: Dmytro Podgornyi <[email protected]>

Author: pasis

src/core/iomux/epoll_wait_call.cpp

@@ -86,15 +86,14 @@ int epoll_wait_call::get_current_events()
     }
 
     xlio_list_t<sockinfo, sockinfo::socket_fd_list_node_offset> socket_fd_list;
+
     lock();
-    int i, ready_rfds = 0, ready_wfds = 0;
-    i = m_n_all_ready_fds;
-    sockinfo *p_socket_object;
-    ep_ready_fd_list_t::iterator iter = m_epfd_info->m_ready_fds.begin();
-    while (iter != m_epfd_info->m_ready_fds.end() && i < m_maxevents) {
-        p_socket_object = *iter;
-        ++iter;
 
+    int i = m_n_all_ready_fds;
+    int ready_rfds = 0;
+    int ready_wfds = 0;
+    sockinfo *p_socket_object = m_epfd_info->m_ready_fds.front();
+    while (p_socket_object && i < m_maxevents) {
         m_events[i].events = 0; // initialize
 
         bool got_event = false;
@@ -147,6 +146,8 @@ int epoll_wait_call::get_current_events()
             socket_fd_list.push_back(p_socket_object);
             ++i;
         }
+
+        p_socket_object = m_epfd_info->m_ready_fds.next(p_socket_object);
     }
 
     m_n_ready_rfds += ready_rfds;

src/core/sock/fd_collection.cpp

@@ -583,11 +583,9 @@ int fd_collection::del_socket(int fd, sockinfo **map_type)
 
 void fd_collection::remove_from_all_epfds(int fd, bool passthrough)
 {
-    epfd_info_list_t::iterator itr;
-
     lock();
-    for (itr = m_epfd_lst.begin(); itr != m_epfd_lst.end(); itr++) {
-        itr->fd_closed(fd, passthrough);
+    for (epfd_info *ep = m_epfd_lst.front(); ep; ep = m_epfd_lst.next(ep)) {
+        ep->fd_closed(fd, passthrough);
     }
     unlock();
 

src/core/sock/sockinfo_tcp.cpp

@@ -1845,10 +1845,8 @@ int sockinfo_tcp::handle_child_FIN(sockinfo_tcp *child_conn)
 {
     lock_tcp_con();
 
-    sock_list_t::iterator conns_iter;
-    for (conns_iter = m_accepted_conns.begin(); conns_iter != m_accepted_conns.end();
-         conns_iter++) {
-        if (*(conns_iter) == child_conn) {
+    for (sockinfo_tcp *conn = m_accepted_conns.front(); conn; conn = m_accepted_conns.next(conn)) {
+        if (conn == child_conn) {
             unlock_tcp_con();
             return 0; // don't close conn, it can be accepted
         }

src/core/sock/sockinfo_ulp.cpp

@@ -1051,14 +1051,13 @@ void sockinfo_tcp_ops_tls::terminate_session_fatal(uint8_t alert_type)
 
 void sockinfo_tcp_ops_tls::copy_by_offset(uint8_t *dst, uint32_t offset, uint32_t len)
 {
-    auto iter = m_rx_bufs.begin();
-    mem_buf_desc_t *pdesc = *iter;
+    mem_buf_desc_t *pdesc = m_rx_bufs.front();
 
     /* Skip leading buffers */
     if (unlikely(pdesc->lwip_pbuf.len <= offset)) {
         while (pdesc && pdesc->lwip_pbuf.len <= offset) {
             offset -= pdesc->lwip_pbuf.len;
-            pdesc = *(++iter);
+            pdesc = m_rx_bufs.next(pdesc);
         }
     }
 
@@ -1071,22 +1070,21 @@ void sockinfo_tcp_ops_tls::copy_by_offset(uint8_t *dst, uint32_t offset, uint32_
         dst += buflen;
         offset = 0;
 
-        pdesc = *(++iter);
+        pdesc = m_rx_bufs.next(pdesc);
     }
 }
 
 /* More efficient method to get 16bit value in the buffer list. */
 uint16_t sockinfo_tcp_ops_tls::offset_to_host16(uint32_t offset)
 {
-    auto iter = m_rx_bufs.begin();
-    mem_buf_desc_t *pdesc = *iter;
+    mem_buf_desc_t *pdesc = m_rx_bufs.front();
     uint16_t res = 0;
 
     /* Skip leading buffers */
     if (unlikely(pdesc->lwip_pbuf.len <= offset)) {
         while (pdesc && pdesc->lwip_pbuf.len <= offset) {
             offset -= pdesc->lwip_pbuf.len;
-            pdesc = *(++iter);
+            pdesc = m_rx_bufs.next(pdesc);
         }
     }
 
@@ -1095,7 +1093,7 @@ uint16_t sockinfo_tcp_ops_tls::offset_to_host16(uint32_t offset)
         ++offset;
         if (unlikely(offset >= pdesc->lwip_pbuf.len)) {
             offset = 0;
-            pdesc = *(++iter);
+            pdesc = m_rx_bufs.next(pdesc);
             if (unlikely(!pdesc)) {
                 return 0;
             }
@@ -1339,7 +1337,6 @@ err_t sockinfo_tcp_ops_tls::recv(struct pbuf *p)
 
     /* The first record is complete - push the payload to application. */
 
-    auto iter = m_rx_bufs.begin();
     struct pbuf *pi;
     struct pbuf *pres = nullptr;
     struct pbuf *ptmp = nullptr;
@@ -1351,7 +1348,7 @@ err_t sockinfo_tcp_ops_tls::recv(struct pbuf *p)
     uint8_t tls_type;
     uint8_t tls_decrypted = 0;
 
-    mem_buf_desc_t *pdesc = *iter;
+    mem_buf_desc_t *pdesc = m_rx_bufs.front();
     tls_type = ((uint8_t *)pdesc->lwip_pbuf.payload)[m_rx_offset];
     if (is_rx_tls13()) {
         /* TLS 1.3 sends record type as the last byte of the payload. */
@@ -1404,7 +1401,7 @@ err_t sockinfo_tcp_ops_tls::recv(struct pbuf *p)
         offset = 0;
 
     next_buffer:
-        pdesc = *(++iter);
+        pdesc = m_rx_bufs.next(pdesc);
     }
 
     int ret = 0;

src/core/util/xlio_list.h

@@ -65,7 +65,6 @@ template <class T, size_t offset(void)> class list_node {
 public:
     /* head must be the first field! */
     struct list_head head;
-    T *obj_ptr;
 
 #if VLIST_DEBUG
     xlio_list_t<T, offset> *parent;
@@ -75,119 +74,23 @@ template <class T, size_t offset(void)> class list_node {
 #endif
 
     list_node()
-        : obj_ptr(NULL)
     {
         this->head.next = &this->head;
         this->head.prev = &this->head;
         VLIST_DEBUG_SET_PARENT(this, NULL);
     }
 
+    T *obj_ptr() { return reinterpret_cast<T *>(reinterpret_cast<uintptr_t>(this) - offset()); }
+
     /* is_list_member - check if the node is already a member in a list. */
     bool is_list_member()
     {
         return this->head.next != &this->head || this->head.prev != &this->head;
     }
 };
 
-template <typename T, size_t offset(void)>
-/* coverity[missing_move_assignment] */
-class list_iterator_t {
-public:
-    using iterator_category = std::random_access_iterator_tag;
-    using value_type = T;
-    using difference_type = std::ptrdiff_t;
-    using pointer = T *;
-    using reference = T &;
-
-    list_iterator_t(T *ptr = NULL)
-        : m_ptr(ptr)
-    {
-    }
-
-    list_iterator_t(const list_iterator_t<T, offset> &iter)
-        : m_ptr(iter.m_ptr)
-    {
-    }
-
-    ~list_iterator_t() {}
-
-    list_iterator_t<T, offset> &operator=(T *ptr)
-    {
-        m_ptr = ptr;
-        return (*this);
-    }
-
-    list_iterator_t<T, offset> &operator=(const list_iterator_t<T, offset> &iter)
-    {
-        m_ptr = iter.m_ptr;
-        return (*this);
-    }
-
-    operator bool() const { return m_ptr ? true : false; }
-
-    bool operator==(const list_iterator_t<T, offset> &iter) const
-    {
-        return m_ptr == iter.getConstPtr();
-    }
-
-    bool operator!=(const list_iterator_t<T, offset> &iter) const
-    {
-        return m_ptr != iter.getConstPtr();
-    }
-
-    list_iterator_t<T, offset> operator++(int)
-    {
-        list_iterator_t<T, offset> iter(*this);
-        increment_ptr();
-        return iter;
-    }
-
-    list_iterator_t<T, offset> &operator++()
-    {
-        increment_ptr();
-        return *this;
-    }
-
-    list_iterator_t<T, offset> operator--(int)
-    {
-        list_iterator_t<T, offset> iter(*this);
-        decrement_ptr();
-        return iter;
-    }
-
-    list_iterator_t<T, offset> &operator--()
-    {
-        decrement_ptr();
-        return *this;
-    }
-
-    T *operator*() { return m_ptr; }
-
-    const T *operator*() const { return m_ptr; }
-
-    T *operator->() { return m_ptr; }
-
-    T *getPtr() const { return m_ptr; }
-
-    const T *getConstPtr() const { return m_ptr; }
-
-private:
-    T *m_ptr;
-
-    inline void increment_ptr()
-    {
-        m_ptr = ((list_node<T, offset> *)GET_NODE(m_ptr, T, offset)->head.next)->obj_ptr;
-    }
-
-    inline void decrement_ptr()
-    {
-        m_ptr = ((list_node<T, offset> *)GET_NODE(m_ptr, T, offset)->head.prev)->obj_ptr;
-    }
-};
-
 template <class T, size_t offset(void)> class xlio_list_t {
 public:
-    typedef list_iterator_t<T, offset> iterator;
     xlio_list_t() { init_list(); }
 
     void set_id(const char *format, ...)
@@ -241,23 +144,27 @@ template <class T, size_t offset(void)> class xlio_list_t {
         if (unlikely(empty())) {
             return NULL;
         }
-        return ((list_node<T, offset> *)m_list.head.next)->obj_ptr;
+        return ((list_node<T, offset> *)m_list.head.next)->obj_ptr();
     }
 
     inline T *back() const
     {
         if (unlikely(empty())) {
             return NULL;
         }
-/* clang analyzer reports:
- * Use of memory after it is freed
- * This issue comes from ~chunk_list_t()
- * Root cause is unknown.
- * TODO: Fix based on root cause instead of supressing
- */
-#ifndef __clang_analyzer__
-        return ((list_node<T, offset> *)m_list.head.prev)->obj_ptr;
-#endif
+        return ((list_node<T, offset> *)m_list.head.prev)->obj_ptr();
+    }
+
+    inline T *next(T *obj) const
+    {
+        list_node<T, offset> *node = (list_node<T, offset> *)GET_NODE(obj, T, offset)->head.next;
+        return node == &m_list ? NULL : node->obj_ptr();
+    }
+
+    inline T *prev(T *obj) const
+    {
+        list_node<T, offset> *node = (list_node<T, offset> *)GET_NODE(obj, T, offset)->head.prev;
+        return node == &m_list ? NULL : node->obj_ptr();
     }
 
     inline void pop_front() { erase(front()); }
@@ -314,7 +221,6 @@ template <class T, size_t offset(void)> class xlio_list_t {
         }
 
         VLIST_DEBUG_SET_PARENT(node_obj, this);
-        node_obj->obj_ptr = obj;
         list_add_tail(&node_obj->head, &m_list.head);
         m_size++;
     }
@@ -332,7 +238,6 @@ template <class T, size_t offset(void)> class xlio_list_t {
         }
 
         VLIST_DEBUG_SET_PARENT(node_obj, this);
-        node_obj->obj_ptr = obj;
         list_add(&node_obj->head, &m_list.head);
         m_size++;
     }
@@ -346,7 +251,7 @@ template <class T, size_t offset(void)> class xlio_list_t {
             for (size_t i = 0; i < index; i++) {
                 ans = ans->next;
             }
-            return ((list_node<T, offset> *)ans)->obj_ptr;
+            return ((list_node<T, offset> *)ans)->obj_ptr();
         }
     }
 
@@ -375,8 +280,8 @@ template <class T, size_t offset(void)> class xlio_list_t {
      * type. Sizes may differ.
      *
      * After the call to this member function, the elements in this container are those which were
-     * in x before the call, and the elements of x are those which were in this. All iterators,
-     * references and pointers remain valid for the swapped objects.
+     * in x before the call, and the elements of x are those which were in this. All references and
+     * pointers remain valid for the swapped objects.
      */
     void swap(xlio_list_t<T, offset> &x)
     {
@@ -386,10 +291,6 @@ template <class T, size_t offset(void)> class xlio_list_t {
         temp_list.move_to_empty(x);
     }
 
-    list_iterator_t<T, offset> begin() { return list_iterator_t<T, offset>(front()); }
-
-    list_iterator_t<T, offset> end() { return list_iterator_t<T, offset>(NULL); }
-
 #if VLIST_DEBUG
     char *list_id() { return (char *)&id; }
 #endif