rewrite some assertions for better readability

mlichvar · mlichvar · commit 711c7c0c8ae3 · 2025-03-06T15:22:36.000+01:00
Some assertions are written as "if (x) assert(0)" to avoid having
the text of a long argument compiled in the binary. Rewrite them
to use a new BRIEF_ASSERT macro to make the condition easier to read in
its non-negated form and make it easier to turn it back to the full-text
assert if needed.
diff --git a/client.c b/client.c
@@ -946,8 +946,7 @@ process_cmd_add_source(CMD_Request *msg, char *line)
       }
 
       msg->data.ntp_source.type = htonl(type);
-      if (strlen(data.name) >= sizeof (msg->data.ntp_source.name))
-        assert(0);
+      BRIEF_ASSERT(strlen(data.name) < sizeof (msg->data.ntp_source.name));
       strncpy((char *)msg->data.ntp_source.name, data.name,
               sizeof (msg->data.ntp_source.name));
       msg->data.ntp_source.port = htonl(data.port);
diff --git a/cmdmon.c b/cmdmon.c
@@ -146,19 +146,17 @@ do_size_checks(void)
     request.command = htons(i);
     request_length = PKL_CommandLength(&request);
     padding_length = PKL_CommandPaddingLength(&request);
-    if (padding_length > MAX_PADDING_LENGTH || padding_length > request_length ||
-        request_length > sizeof (CMD_Request) ||
-        (request_length && request_length < offsetof(CMD_Request, data)))
-      assert(0);
+    BRIEF_ASSERT(padding_length <= MAX_PADDING_LENGTH && padding_length <= request_length &&
+                 request_length <= sizeof (CMD_Request) &&
+                 (request_length == 0 || request_length >= offsetof(CMD_Request, data)));
   }
 
   for (i = 1; i < N_REPLY_TYPES; i++) {
     reply.reply = htons(i);
     reply.status = STT_SUCCESS;
     reply_length = PKL_ReplyLength(&reply);
-    if ((reply_length && reply_length < offsetof(CMD_Reply, data)) ||
-        reply_length > sizeof (CMD_Reply))
-      assert(0);
+    BRIEF_ASSERT((reply_length == 0 || reply_length >= offsetof(CMD_Reply, data)) &&
+                 reply_length <= sizeof (CMD_Reply));
   }
 }
 
diff --git a/conf.c b/conf.c
@@ -2828,8 +2828,7 @@ CNF_GetNtsTrustedCertsPaths(const char ***paths, uint32_t **ids)
   *paths = ARR_GetElements(nts_trusted_certs_paths);
   *ids = ARR_GetElements(nts_trusted_certs_ids);
 
-  if (ARR_GetSize(nts_trusted_certs_paths) != ARR_GetSize(nts_trusted_certs_ids))
-    assert(0);
+  BRIEF_ASSERT(ARR_GetSize(nts_trusted_certs_paths) == ARR_GetSize(nts_trusted_certs_ids));
 
   return ARR_GetSize(nts_trusted_certs_paths);
 }
diff --git a/local.c b/local.c
@@ -184,10 +184,8 @@ void
 LCL_Finalise(void)
 {
   /* Make sure all handlers have been removed */
-  if (change_list.next != &change_list)
-    assert(0);
-  if (dispersion_notify_list.next != &dispersion_notify_list)
-    assert(0);
+  BRIEF_ASSERT(change_list.next == &change_list);
+  BRIEF_ASSERT(dispersion_notify_list.next == &dispersion_notify_list);
 }
 
 /* ================================================== */
@@ -225,9 +223,7 @@ LCL_AddParameterChangeHandler(LCL_ParameterChangeHandler handler, void *anything
 
   /* Check that the handler is not already registered */
   for (ptr = change_list.next; ptr != &change_list; ptr = ptr->next) {
-    if (!(ptr->handler != handler || ptr->anything != anything)) {
-      assert(0);
-    }
+    BRIEF_ASSERT(ptr->handler != handler || ptr->anything != anything);
   }
 
   new_entry = MallocNew(ChangeListEntry);
@@ -301,9 +297,7 @@ LCL_AddDispersionNotifyHandler(LCL_DispersionNotifyHandler handler, void *anythi
 
   /* Check that the handler is not already registered */
   for (ptr = dispersion_notify_list.next; ptr != &dispersion_notify_list; ptr = ptr->next) {
-    if (!(ptr->handler != handler || ptr->anything != anything)) {
-      assert(0);
-    }
+    BRIEF_ASSERT(ptr->handler != handler || ptr->anything != anything);
   }
 
   new_entry = MallocNew(DispersionNotifyListEntry);
diff --git a/ntp_core.c b/ntp_core.c
@@ -2337,9 +2337,8 @@ process_response(NCR_Instance inst, int saved, NTP_Local_Address *local_addr,
     inst->valid_rx = 1;
   }
 
-  if ((unsigned int)local_receive.source >= sizeof (tss_chars) ||
-      (unsigned int)local_transmit.source >= sizeof (tss_chars))
-    assert(0);
+  BRIEF_ASSERT((unsigned int)local_receive.source < sizeof (tss_chars) &&
+               (unsigned int)local_transmit.source < sizeof (tss_chars));
 
   DEBUG_LOG("NTP packet lvm=%o stratum=%d poll=%d prec=%d root_delay=%.9f root_disp=%.9f refid=%"PRIx32" [%s]",
             message->lvm, message->stratum, message->poll, message->precision,
diff --git a/ntp_sources.c b/ntp_sources.c
@@ -457,9 +457,8 @@ change_source_address(NTP_Remote_Address *old_addr, NTP_Remote_Address *new_addr
   if (replacement)
     record->resolved_addr = new_addr->ip_addr;
 
-  if (record->remote_addr != NCR_GetRemoteAddress(record->data) ||
-      UTI_CompareIPs(&record->remote_addr->ip_addr, &new_addr->ip_addr, NULL) != 0)
-    assert(0);
+  BRIEF_ASSERT(record->remote_addr == NCR_GetRemoteAddress(record->data) &&
+               UTI_CompareIPs(&record->remote_addr->ip_addr, &new_addr->ip_addr, NULL) == 0);
 
   if (!UTI_IsIPReal(&old_addr->ip_addr) && UTI_IsIPReal(&new_addr->ip_addr)) {
     if (auto_start_sources)
diff --git a/nts_ke_server.c b/nts_ke_server.c
@@ -520,8 +520,7 @@ generate_key(int index)
   ServerKey *key;
   int key_length;
 
-  if (index < 0 || index >= MAX_SERVER_KEYS)
-    assert(0);
+  BRIEF_ASSERT(index >= 0 && index < MAX_SERVER_KEYS);
 
   /* Prefer AES-128-GCM-SIV if available.  Note that if older keys loaded
      from ntsdumpdir use a different algorithm, responding to NTP requests
@@ -534,8 +533,7 @@ generate_key(int index)
   key = &server_keys[index];
 
   key_length = SIV_GetKeyLength(algorithm);
-  if (key_length > sizeof (key->key))
-    assert(0);
+  BRIEF_ASSERT(key_length <= sizeof (key->key));
 
   UTI_GetRandomBytesUrandom(key->key, key_length);
   memset(key->key + key_length, 0, sizeof (key->key) - key_length);
@@ -961,8 +959,7 @@ NKS_GenerateCookie(NKE_Context *context, NKE_Cookie *cookie)
   header->key_id = htonl(key->id);
 
   nonce = cookie->cookie + sizeof (*header);
-  if (key->nonce_length > sizeof (cookie->cookie) - sizeof (*header))
-    assert(0);
+  BRIEF_ASSERT(key->nonce_length <= sizeof (cookie->cookie) - sizeof (*header));
   UTI_GetRandomBytes(nonce, key->nonce_length);
 
   plaintext_length = context->c2s.length + context->s2c.length;
diff --git a/nts_ke_session.c b/nts_ke_session.c
@@ -663,8 +663,7 @@ create_credentials(const char **certs, const char **keys, int n_certs_keys,
     goto error;
 
   if (certs && keys) {
-    if (trusted_certs || trusted_certs_ids)
-      assert(0);
+    BRIEF_ASSERT(!trusted_certs && !trusted_certs_ids);
 
     for (i = 0; i < n_certs_keys; i++) {
       if (!UTI_CheckFilePermissions(keys[i], 0771))
@@ -675,8 +674,7 @@ create_credentials(const char **certs, const char **keys, int n_certs_keys,
         goto error;
     }
   } else {
-    if (certs || keys || n_certs_keys > 0)
-      assert(0);
+    BRIEF_ASSERT(!certs && !keys && n_certs_keys <= 0);
 
     if (trusted_cert_set == 0 && !CNF_GetNoSystemCert()) {
       r = gnutls_certificate_set_x509_system_trust(credentials);
diff --git a/nts_ntp_auth.c b/nts_ntp_auth.c
@@ -104,9 +104,8 @@ NNA_GenerateAuthEF(NTP_Packet *packet, NTP_PacketInfo *info, SIV_Instance siv,
   body = (unsigned char *)(header + 1);
   ciphertext = body + nonce_length + nonce_padding;
 
-  if ((unsigned char *)header + auth_length !=
-      ciphertext + ciphertext_length + ciphertext_padding + additional_padding)
-    assert(0);
+  BRIEF_ASSERT((unsigned char *)header + auth_length ==
+               ciphertext + ciphertext_length + ciphertext_padding + additional_padding);
 
   memcpy(body, nonce, nonce_length);
   memset(body + nonce_length, 0, nonce_padding);
diff --git a/nts_ntp_server.c b/nts_ntp_server.c
@@ -259,8 +259,7 @@ NNS_GenerateResponseAuth(NTP_Packet *request, NTP_PacketInfo *req_info,
 
   /* Make sure this is a response to the request from the last call
      of NNS_CheckRequestAuth() */
-  if (UTI_CompareNtp64(&server->req_tx, &request->transmit_ts) != 0)
-    assert(0);
+  BRIEF_ASSERT(UTI_CompareNtp64(&server->req_tx, &request->transmit_ts) == 0);
 
   for (parsed = NTP_HEADER_LENGTH; parsed < req_info->length; parsed += ef_length) {
     if (!NEF_ParseField(request, req_info->length, parsed,
diff --git a/privops.c b/privops.c
@@ -547,9 +547,9 @@ PRV_BindSocket(int sock, struct sockaddr *address, socklen_t address_len)
   PrvResponse res;
 
   SCK_SockaddrToIPSockAddr(address, address_len, &ip_saddr);
-  if (ip_saddr.port != 0 && ip_saddr.port != CNF_GetNTPPort() &&
-      ip_saddr.port != CNF_GetAcquisitionPort() && ip_saddr.port != CNF_GetPtpPort())
-    assert(0);
+  BRIEF_ASSERT(ip_saddr.port == 0 || ip_saddr.port == CNF_GetNTPPort() ||
+               ip_saddr.port == CNF_GetAcquisitionPort() ||
+               ip_saddr.port == CNF_GetPtpPort());
 
   if (!have_helper())
     return bind(sock, address, address_len);
diff --git a/quantiles.c b/quantiles.c
@@ -62,9 +62,8 @@ QNT_CreateInstance(int min_k, int max_k, int q, int repeat,
   QNT_Instance inst;
   long seed;
 
-  if (q < 2 || min_k > max_k || min_k < 1 || max_k >= q ||
-      repeat < 1 || repeat > MAX_REPEAT || min_step <= 0.0 || large_step_delay < 0)
-    assert(0);
+  BRIEF_ASSERT(q >= 2 && min_k <= max_k && min_k >= 1 && max_k < q && repeat >= 1 &&
+               repeat <= MAX_REPEAT && min_step > 0.0 && large_step_delay >= 0);
 
   inst = MallocNew(struct QNT_Instance_Record);
   inst->n_quants = (max_k - min_k + 1) * repeat;
@@ -117,8 +116,7 @@ insert_initial_value(QNT_Instance inst, double value)
 {
   int i, j, r = inst->repeat;
 
-  if (inst->n_set * r >= inst->n_quants)
-    assert(0);
+  BRIEF_ASSERT(inst->n_set * r < inst->n_quants);
 
   /* Keep the initial estimates repeated and ordered */
   for (i = inst->n_set; i > 0 && inst->quants[(i - 1) * r].est > value; i--) {
@@ -225,8 +223,7 @@ QNT_GetQuantile(QNT_Instance inst, int k)
   double estimates[MAX_REPEAT];
   int i;
 
-  if (k < inst->min_k || (k - inst->min_k) * inst->repeat >= inst->n_quants)
-    assert(0);
+  BRIEF_ASSERT(k >= inst->min_k && (k - inst->min_k) * inst->repeat < inst->n_quants);
 
   for (i = 0; i < inst->repeat; i++)
     estimates[i] = inst->quants[(k - inst->min_k) * inst->repeat + i].est;
diff --git a/siv_nettle.c b/siv_nettle.c
@@ -35,6 +35,7 @@
 
 #include "memory.h"
 #include "siv.h"
+#include "util.h"
 
 struct SIV_Instance_Record {
   SIV_Algorithm algorithm;
@@ -158,8 +159,7 @@ SIV_GetMaxNonceLength(SIV_Instance instance)
 int
 SIV_GetTagLength(SIV_Instance instance)
 {
-  if (instance->tag_length < 1 || instance->tag_length > SIV_MAX_TAG_LENGTH)
-    assert(0);
+  BRIEF_ASSERT(instance->tag_length >= 1 && instance->tag_length <= SIV_MAX_TAG_LENGTH);
   return instance->tag_length;
 }
 
diff --git a/socket.c b/socket.c
@@ -1061,9 +1061,8 @@ receive_messages(int sock_fd, int flags, int max_messages, int *num_messages)
   n = ARR_GetSize(recv_headers);
   n = MIN(n, max_messages);
 
-  if (n < 1 || n > MAX_RECV_MESSAGES ||
-      n > ARR_GetSize(recv_messages) || n > ARR_GetSize(recv_sck_messages))
-    assert(0);
+  BRIEF_ASSERT(n >= 1 && n <= MAX_RECV_MESSAGES &&
+               n <= ARR_GetSize(recv_messages) && n <= ARR_GetSize(recv_sck_messages));
 
   recv_flags = get_recv_flags(flags);
 
diff --git a/sources.c b/sources.c
@@ -322,9 +322,8 @@ void SRC_DestroyInstance(SRC_Instance instance)
     last_updated_inst = NULL;
 
   assert(initialised);
-  if (instance->index < 0 || instance->index >= n_sources ||
-      instance != sources[instance->index])
-    assert(0);
+  BRIEF_ASSERT(instance->index >= 0 && instance->index < n_sources &&
+               instance == sources[instance->index]);
 
   SST_DeleteInstance(instance->stats);
   dead_index = instance->index;
@@ -763,8 +762,7 @@ mark_source(SRC_Instance inst, SRC_Status status)
 {
   set_source_status(inst, status);
 
-  if (status < SRC_OK || status >= sizeof (inst->reported_status))
-    assert(0);
+  BRIEF_ASSERT(status >= SRC_OK && status < sizeof (inst->reported_status));
 
   if (!inst->reported_status[status]) {
     switch (status) {
diff --git a/util.h b/util.h
@@ -272,4 +272,12 @@ extern int UTI_SplitString(char *string, char **words, int max_saved_words);
 
 #define SQUARE(x) ((x) * (x))
 
+/* Macro to make an assertion with the text of a long argument replaced
+   with "0" to avoid bloating the compiled binary */
+#ifdef NDEBUG
+#define BRIEF_ASSERT(a)
+#else
+#define BRIEF_ASSERT(a) if (!(a)) assert(0)
+#endif
+
 #endif /* GOT_UTIL_H */

Original file line number	Diff line number	Diff line change
`@@ -946,8 +946,7 @@ process_cmd_add_source(CMD_Request msg, char line)`
`946`	`946`	`}`
`947`	`947`
`948`	`948`	`msg->data.ntp_source.type = htonl(type);`
`949`		`- if (strlen(data.name) >= sizeof (msg->data.ntp_source.name))`
`950`		`- assert(0);`
	`949`	`+ BRIEF_ASSERT(strlen(data.name) < sizeof (msg->data.ntp_source.name));`
`951`	`950`	`strncpy((char *)msg->data.ntp_source.name, data.name,`
`952`	`951`	`sizeof (msg->data.ntp_source.name));`
`953`	`952`	`msg->data.ntp_source.port = htonl(data.port);`
Original file line number	Diff line number	Diff line change
`@@ -2828,8 +2828,7 @@ CNF_GetNtsTrustedCertsPaths(const char *paths, uint32_t ids)`
`2828`	`2828`	`*paths = ARR_GetElements(nts_trusted_certs_paths);`
`2829`	`2829`	`*ids = ARR_GetElements(nts_trusted_certs_ids);`
`2830`	`2830`
`2831`		`- if (ARR_GetSize(nts_trusted_certs_paths) != ARR_GetSize(nts_trusted_certs_ids))`
`2832`		`- assert(0);`
	`2831`	`+ BRIEF_ASSERT(ARR_GetSize(nts_trusted_certs_paths) == ARR_GetSize(nts_trusted_certs_ids));`
`2833`	`2832`
`2834`	`2833`	`return ARR_GetSize(nts_trusted_certs_paths);`
`2835`	`2834`	`}`