Repository: valkey-io/valkey
File: src/connection.h (C/C++)

Code before:
    int (*has_pending_data)(void);
    int (*process_pending_data)(void);

    /* Postpone update state - with IO threads & TLS we don't want the IO threads to update the event loop events - let
     * the main-thread do it */
    void (*postpone_update_state)(struct connection *conn, int);
    /* Called by the main-thread */
    void (*update_state)(struct connection *conn);

    /* TLS specified methods */
    sds (*get_peer_cert)(struct connection *conn);

    /* Get peer username based on connection type */
    sds (*get_peer_username)(connection *conn);

    /* Miscellaneous */
    int (*connIntegrityChecked)(void); // return 1 if connection type has built-in integrity checks
} ConnectionType;

struct connection {
    ConnectionType *type;
    ConnectionState state;
    int last_errno;
    int fd;
    char client_ip[NET_IP_STR_LEN];
    int client_port;
    short int flags;
    short int refs;
    unsigned short int iovcnt;
    void *private_data;
    ConnectionCallbackFunc conn_handler;
    ConnectionCallbackFunc write_handler;
    ConnectionCallbackFunc read_handler;
};

#define CONFIG_BINDADDR_MAX 16

/* Setup a listener by a connection type */
struct connListener {
    int fd[CONFIG_BINDADDR_MAX];
    int count;
    char **bindaddr;
    int bindaddr_count;
    int port;
    ConnectionType *ct;
    void *priv; /* used by connection type specified data */
};

/* The connection module does not deal with listening and accepting sockets,
 * so we assume we have a socket when an incoming connection is created.
 *

Review comment (bug):
This breaks the connection abstraction.

`connection` is used widely throughout Valkey (not just in cluster_legacy.c).  Concerns:
* Connections are abstract.  Not all connections may have an IP address.  This adds fields for all connections, regardless of type.
* These fields are only initialized and used within cluster_legacy.c.  This makes them extraneous/undefined in all other code.
* These are only used for a debug log - potentially adding over 3MB with 65,000 connections.

Suggest: Eliminate these fields, and use the `connAddr()` function to retrieve the IP/port.



Code after:
    int (*has_pending_data)(void);
    int (*process_pending_data)(void);

    /* Postpone update state - with IO threads & TLS we don't want the IO threads to update the event loop events - let
     * the main-thread do it */
    void (*postpone_update_state)(struct connection *conn, int);
    /* Called by the main-thread */
    void (*update_state)(struct connection *conn);

    /* TLS specified methods */
    sds (*get_peer_cert)(struct connection *conn);

    /* Get peer username based on connection type */
    sds (*get_peer_username)(connection *conn);

    /* Miscellaneous */
    int (*connIntegrityChecked)(void); // return 1 if connection type has built-in integrity checks
} ConnectionType;

struct connection {
    ConnectionType *type;
    ConnectionState state;
    int last_errno;
    int fd;
    short int flags;
    short int refs;
    unsigned short int iovcnt;
    void *private_data;
    ConnectionCallbackFunc conn_handler;
    ConnectionCallbackFunc write_handler;
    ConnectionCallbackFunc read_handler;
};

#define CONFIG_BINDADDR_MAX 16

/* Setup a listener by a connection type */
struct connListener {
    int fd[CONFIG_BINDADDR_MAX];
    int count;
    char **bindaddr;
    int bindaddr_count;
    int port;
    ConnectionType *ct;
    void *priv; /* used by connection type specified data */
};

/* The connection module does not deal with listening and accepting sockets,
 * so we assume we have a socket when an incoming connection is created.
 *
 * The fd supplied should therefore be associated with an already accept()ed
 * socket.