commit 4e269dc: Merge pull request #3313 from arkamar/hiredis

[GitHub]

Author: Vsevolod Stakhov
Date: 2020-04-24 15:08:40 +0100
URL: https://github.com/rspamd/rspamd/commit/4e269dc2f8c58fe923b42717b7ea5bcc55bde25d (HEAD -> master)

Merge pull request #3313 from arkamar/hiredis
Revert "[Fix] Fix hiredis stupidity" and use the hiredis api as intended

 contrib/hiredis/async.c    | 8 +++-----
 contrib/hiredis/async.h    | 6 ++----
 src/libserver/redis_pool.c | 9 ++++-----
 3 files changed, 9 insertions(+), 14 deletions(-)

diff --combined contrib/hiredis/async.h
index 40a5a49c8,59cbf469b..f19139c6f
--- a/contrib/hiredis/async.h
+++ b/contrib/hiredis/async.h
@@@ -54,7 -54,7 +54,7 @@@ typedef struct redisCallbackList 
  } redisCallbackList;
  
  /* Connection callback prototypes */
- typedef void (redisDisconnectCallback)(const struct redisAsyncContext*, int status, void *cbdata);
+ typedef void (redisDisconnectCallback)(const struct redisAsyncContext*, int status);
  typedef void (redisConnectCallback)(const struct redisAsyncContext*, int status);
  
  /* Context for an async connection to Redis */
@@@ -85,8 -85,6 +85,6 @@@ typedef struct redisAsyncContext 
      /* Called when either the connection is terminated due to an error or per
       * user request. The status is set accordingly (REDIS_OK, REDIS_ERR). */
      redisDisconnectCallback *onDisconnect;
-     /* Hiredis is just brain-damaged here, need to fix it */
-     void *disconnectCbdata;
  
      /* Called when the first write event was received. */
      redisConnectCallback *onConnect;
@@@ -109,7 -107,7 +107,7 @@@ redisAsyncContext *redisAsyncConnectBin
                                                    const char *source_addr);
  redisAsyncContext *redisAsyncConnectUnix(const char *path);
  int redisAsyncSetConnectCallback(redisAsyncContext *ac, redisConnectCallback *fn);
- int redisAsyncSetDisconnectCallback(redisAsyncContext *ac, redisDisconnectCallback *fn, void *cbdata);
+ int redisAsyncSetDisconnectCallback(redisAsyncContext *ac, redisDisconnectCallback *fn);
  void redisAsyncDisconnect(redisAsyncContext *ac);
  void redisAsyncFree(redisAsyncContext *ac);
  
@@@ -119,13 -117,7 +117,13 @@@ void redisAsyncHandleWrite(redisAsyncCo
  
  /* Command functions for an async context. Write the command to the
   * output buffer and register the provided callback. */
 +#ifdef __GNUC__
 +__attribute__((format(printf, 4, 0)))
 +#endif
  int redisvAsyncCommand(redisAsyncContext *ac, redisCallbackFn *fn, void *privdata, const char *format, va_list ap);
 +#ifdef __GNUC__
 +__attribute__((format(printf, 4, 5)))
 +#endif
  int redisAsyncCommand(redisAsyncContext *ac, redisCallbackFn *fn, void *privdata, const char *format, ...);
  int redisAsyncCommandArgv(redisAsyncContext *ac, redisCallbackFn *fn, void *privdata, int argc, const char **argv, const size_t *argvlen);
  int redisAsyncFormattedCommand(redisAsyncContext *ac, redisCallbackFn *fn, void *privdata, const char *cmd, size_t len);
diff --combined src/libserver/redis_pool.c
index 4d1a18676,2d2510753..7e835013c
--- a/src/libserver/redis_pool.c
+++ b/src/libserver/redis_pool.c
@@@ -258,10 -258,9 +258,9 @@@ rspamd_redis_pool_schedule_timeout (str
  }
  
  static void
- rspamd_redis_pool_on_disconnect (const struct redisAsyncContext *ac, int status,
- 		void *ud)
+ rspamd_redis_pool_on_disconnect (const struct redisAsyncContext *ac, int status)
  {
- 	struct rspamd_redis_pool_connection *conn = ud;
+ 	struct rspamd_redis_pool_connection *conn = ac->data;
  
  	/*
  	 * Here, we know that redis itself will free this connection
@@@ -313,13 -312,13 +312,13 @@@ rspamd_redis_pool_new_connection (struc
  			g_hash_table_insert (elt->pool->elts_by_ctx, ctx, conn);
  			g_queue_push_head_link (elt->active, conn->entry);
  			conn->ctx = ctx;
+ 			ctx->data = conn;
  			rspamd_random_hex (conn->tag, sizeof (conn->tag));
  			REF_INIT_RETAIN (conn, rspamd_redis_pool_conn_dtor);
  			msg_debug_rpool ("created new connection to %s:%d: %p", ip, port, ctx);
  
  			redisLibevAttach (pool->event_loop, ctx);
- 			redisAsyncSetDisconnectCallback (ctx, rspamd_redis_pool_on_disconnect,
- 					conn);
+ 			redisAsyncSetDisconnectCallback (ctx, rspamd_redis_pool_on_disconnect);
  
  			if (password) {
  				redisAsyncCommand (ctx, NULL, NULL,
@@@ -401,30 -400,11 +400,30 @@@ rspamd_redis_pool_connect (struct rspam
  			g_assert (conn->state != RSPAMD_REDIS_POOL_CONN_ACTIVE);
  
  			if (conn->ctx->err == REDIS_OK) {
 -				ev_timer_stop (elt->pool->event_loop, &conn->timeout);
 -				conn->state = RSPAMD_REDIS_POOL_CONN_ACTIVE;
 -				g_queue_push_tail_link (elt->active, conn_entry);
 -				msg_debug_rpool ("reused existing connection to %s:%d: %p",
 -						ip, port, conn->ctx);
 +				/* Also check SO_ERROR */
 +				gint err;
 +				socklen_t len = sizeof (gint);
 +
 +				if (getsockopt (conn->ctx->c.fd, SOL_SOCKET, SO_ERROR,
 +						(void *) &err, &len) == -1) {
 +					err = errno;
 +				}
 +
 +				if (err != 0) {
 +					g_list_free (conn->entry);
 +					conn->entry = NULL;
 +					REF_RELEASE (conn);
 +					conn = rspamd_redis_pool_new_connection (pool, elt,
 +							db, password, ip, port);
 +				}
 +				else {
 +
 +					ev_timer_stop (elt->pool->event_loop, &conn->timeout);
 +					conn->state = RSPAMD_REDIS_POOL_CONN_ACTIVE;
 +					g_queue_push_tail_link (elt->active, conn_entry);
 +					msg_debug_rpool ("reused existing connection to %s:%d: %p",
 +							ip, port, conn->ctx);
 +				}
  			}
  			else {
  				g_list_free (conn->entry);