From 31c3172d9b205d12216c27493806b6ac2b4986bd Mon Sep 17 00:00:00 2001
From: Madelyn Olson <34459052+madolson@users.noreply.github.com>
Date: Mon, 2 Oct 2023 18:58:44 -0700
Subject: [PATCH] Better standardize around assertions (#12539)

We use the C standard assert() in various places in the codebase, which requires NDEBUG to be undefined. We introduced the redisassert.h file in order to allow low level files to access the assert that maps to serverPanic, but this was only applied tactically and is not available broadly.

This PR removes all usage of the standard library asserts and replaces them with an assert that maps to serverPanic. It makes us immune to accidentally setting the NDEBUG flag preventing assertions. I also marked marked the server asserts as "likely" to not execute. I spot checked various points in the code, and it didn't change the code layout on my x86 mac, but it is more consistent with redisassert.h and seems more correct overall.
---
 src/defrag.c    | 1 -
 src/memtest.c   | 2 +-
 src/monotonic.c | 5 +----
 src/rax.c       | 2 +-
 src/sds.c       | 2 +-
 src/server.c    | 1 -
 src/server.h    | 5 ++---
 src/siphash.c   | 1 -
 src/zmalloc.c   | 1 -
 9 files changed, 6 insertions(+), 14 deletions(-)

diff --git a/src/defrag.c b/src/defrag.c
index ff63cf8fd..af403398a 100644
--- a/src/defrag.c
+++ b/src/defrag.c
@@ -36,7 +36,6 @@
 #include "server.h"
 #include "cluster.h"
 #include <time.h>
-#include <assert.h>
 #include <stddef.h>
 
 #ifdef HAVE_DEFRAG
diff --git a/src/memtest.c b/src/memtest.c
index 1ca4b82cf..ee1ba92f1 100644
--- a/src/memtest.c
+++ b/src/memtest.c
@@ -30,7 +30,6 @@
 #include <stdlib.h>
 #include <stdio.h>
 #include <string.h>
-#include <assert.h>
 #include <limits.h>
 #include <errno.h>
 #include <termios.h>
@@ -39,6 +38,7 @@
 #include <stropts.h>
 #endif
 #include "config.h"
+#include "redisassert.h"
 
 #if (ULONG_MAX == 4294967295UL)
 #define MEMTEST_32BIT
diff --git a/src/monotonic.c b/src/monotonic.c
index 1d71962f3..6da03677b 100644
--- a/src/monotonic.c
+++ b/src/monotonic.c
@@ -3,10 +3,7 @@
 #include <stdlib.h>
 #include <stdio.h>
 #include <time.h>
-
-#undef NDEBUG
-#include <assert.h>
-
+#include "redisassert.h"
 
 /* The function pointer for clock retrieval.  */
 monotime (*getMonotonicUs)(void) = NULL;
diff --git a/src/rax.c b/src/rax.c
index 287f9855d..304b26fe8 100644
--- a/src/rax.c
+++ b/src/rax.c
@@ -32,11 +32,11 @@
 
 #include <stdlib.h>
 #include <string.h>
-#include <assert.h>
 #include <stdio.h>
 #include <errno.h>
 #include <math.h>
 #include "rax.h"
+#include "redisassert.h"
 
 #ifndef RAX_MALLOC_INCLUDE
 #define RAX_MALLOC_INCLUDE "rax_malloc.h"
diff --git a/src/sds.c b/src/sds.c
index e383e3caa..f5383f90e 100644
--- a/src/sds.c
+++ b/src/sds.c
@@ -34,8 +34,8 @@
 #include <stdlib.h>
 #include <string.h>
 #include <ctype.h>
-#include <assert.h>
 #include <limits.h>
+#include "redisassert.h"
 #include "sds.h"
 #include "sdsalloc.h"
 #include "util.h"
diff --git a/src/server.c b/src/server.c
index ae9004a8c..b164b8cbb 100644
--- a/src/server.c
+++ b/src/server.c
@@ -45,7 +45,6 @@
 #include <signal.h>
 #include <sys/wait.h>
 #include <errno.h>
-#include <assert.h>
 #include <ctype.h>
 #include <stdarg.h>
 #include <arpa/inet.h>
diff --git a/src/server.h b/src/server.h
index ac324eb6c..27803573a 100644
--- a/src/server.h
+++ b/src/server.h
@@ -37,7 +37,6 @@
 #include "atomicvar.h"
 #include "commands.h"
 
-#include <assert.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <stddef.h>
@@ -669,8 +668,8 @@ typedef enum {
 #define run_with_period(_ms_) if (((_ms_) <= 1000/server.hz) || !(server.cronloops%((_ms_)/(1000/server.hz))))
 
 /* We can print the stacktrace, so our assert is defined this way: */
-#define serverAssertWithInfo(_c,_o,_e) ((_e)?(void)0 : (_serverAssertWithInfo(_c,_o,#_e,__FILE__,__LINE__),redis_unreachable()))
-#define serverAssert(_e) ((_e)?(void)0 : (_serverAssert(#_e,__FILE__,__LINE__),redis_unreachable()))
+#define serverAssertWithInfo(_c,_o,_e) (likely(_e)?(void)0 : (_serverAssertWithInfo(_c,_o,#_e,__FILE__,__LINE__),redis_unreachable()))
+#define serverAssert(_e) (likely(_e)?(void)0 : (_serverAssert(#_e,__FILE__,__LINE__),redis_unreachable()))
 #define serverPanic(...) _serverPanic(__FILE__,__LINE__,__VA_ARGS__),redis_unreachable()
 
 /* latency histogram per command init settings */
diff --git a/src/siphash.c b/src/siphash.c
index 2713d89ee..a62d5c061 100644
--- a/src/siphash.c
+++ b/src/siphash.c
@@ -39,7 +39,6 @@
       the function in the new form (returning an uint64_t) using just the
       relevant test vector.
  */
-#include <assert.h>
 #include <stdint.h>
 #include <stdio.h>
 #include <string.h>
diff --git a/src/zmalloc.c b/src/zmalloc.c
index 3241a70fb..343d730a2 100644
--- a/src/zmalloc.c
+++ b/src/zmalloc.c
@@ -36,7 +36,6 @@
 #include <stdlib.h>
 #include <stdint.h>
 #include <unistd.h>
-#include <assert.h>
 
 #ifdef __linux__
 #include <sys/mman.h>