On Wed, 16 Nov 2022 15:41:45 +1100
David Gibson <david(a)gibson.dropbear.id.au> wrote:
The macro CONN_OR_NULL() is used to look up
connections by index with
bounds checking. Replace it with an inline function, which means:
- Better type checking
- No danger of multiple evaluation of an @index with side effects
Also add a helper to perform the reverse translation: from connection
pointer to index. Introduce a macro for this which will make later
cleanups easier and safer.
Ah, yes, much better, agreed. Just two things here:
Signed-off-by: David Gibson
<david(a)gibson.dropbear.id.au>
---
tcp.c | 83 ++++++++++++++++++++++++++++++++---------------------------
1 file changed, 45 insertions(+), 38 deletions(-)
diff --git a/tcp.c b/tcp.c
index d043123..4e56a6c 100644
--- a/tcp.c
+++ b/tcp.c
@@ -518,14 +518,6 @@ struct tcp_conn {
(conn->events & (SOCK_FIN_RCVD | TAP_FIN_RCVD)))
#define CONN_HAS(conn, set) ((conn->events & (set)) == (set))
-#define CONN(index) (tc + (index))
-
-/* We probably don't want to use gcc statement expressions (for portability), so
- * use this only after well-defined sequence points (no pre-/post-increments).
- */
-#define CONN_OR_NULL(index) \
- (((int)(index) >= 0 && (index) < TCP_MAX_CONNS) ? (tc + (index)) : NULL)
-
static const char *tcp_event_str[] __attribute((__unused__)) = {
"SOCK_ACCEPTED", "TAP_SYN_RCVD", "ESTABLISHED",
"TAP_SYN_ACK_SENT",
@@ -705,6 +697,21 @@ static size_t tcp6_l2_flags_buf_bytes;
/* TCP connections */
static struct tcp_conn tc[TCP_MAX_CONNS];
+#define CONN(index) (tc + (index))
+#define CONN_IDX(conn) ((conn) - tc)
+
+/** conn_at_idx() - Find a connection by index, if present
+ * @index: Index of connection to lookup
+ *
+ * Return: Pointer to connection, or NULL if @index is out of bounds
Return: pointer [...]
+ */
+static inline struct tcp_conn *conn_at_idx(int index)
The CONN_OR_NULL name made it very explicit that the pointer obtained
there could be NULL. On the other hand I find conn_at_idx() more
descriptive. But maybe conn_or_null() would be "safer". I don't really
have a preference.
I see your point, but on balance I think I marginally prefer
conn_at_idx(), so I've left it.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!