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 boundsReturn: 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. -- Stefano