]> git.ipfire.org Git - thirdparty/chrony.git/commitdiff
rewrite some assertions for better readability
authorMiroslav Lichvar <mlichvar@redhat.com>
Wed, 5 Mar 2025 11:28:46 +0000 (12:28 +0100)
committerMiroslav Lichvar <mlichvar@redhat.com>
Thu, 6 Mar 2025 14:22:36 +0000 (15:22 +0100)
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.

16 files changed:
client.c
cmdmon.c
conf.c
local.c
ntp_core.c
ntp_sources.c
nts_ke_server.c
nts_ke_session.c
nts_ntp_auth.c
nts_ntp_server.c
privops.c
quantiles.c
siv_nettle.c
socket.c
sources.c
util.h

index 6bbce140e77c0448d6a87b4a9aabd7566cf5c869..787559a961e816c70f9889f5cadee87054c55feb 100644 (file)
--- 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);
index 57867c7debd96bb901f5258f8f8d092350ea2751..18d955f78e33616f2638963697c8a46bc01b4658 100644 (file)
--- 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
index 5f60048042e3b5631c3d9c6ced341b83802d078e..7c84e13bacf72ec282b7bb42cb130b5b08d303b0 100644 (file)
--- 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
index b32d0c6bcdaa12a6a66f95aa9990ff2e8b9f51d9..785559471a8bf7e581ebb9170bc98918a2dde2a5 100644 (file)
--- 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);
index 19b70a9b0c876b35edbffd03fb5dae22936d2c0a..fa6acc34c60d2b1e5733915431500377d1c0e1d1 100644 (file)
@@ -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,
index f117e33b71b47f472af197cfc485f71b098adcf1..7bf0889440f60d0c7d4495de5b1cbf303ab23a3e 100644 (file)
@@ -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)
index 6dd6251f3a0ef97df8bbe624f4698d29c5e4ff2d..26cc0cf04ee8d59309db478ba5732a44bf98f775 100644 (file)
@@ -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;
index 3d7880554948cdaa2831d406a6b3aa1fb50774c8..c5f49d3d1654c40c97719eb55cded93d0e7af55c 100644 (file)
@@ -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);
index b92c406b2a9c062c4446ebe915a4022ff5d40009..7e5aca29589f87431c14a45ce9ce5ddf5aabc5fe 100644 (file)
@@ -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);
index 5d46c29619b036584dccfc4875503f3e91a0d49b..ac478f132fdac4a0c402692023045c4be4c790d5 100644 (file)
@@ -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,
index e1c25e0ca7aef8355171d9403aa30e3bd460a1ff..3bc76d140c8d59df01a3e9e67d7237d485e49906 100644 (file)
--- 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);
index 3f0800dd40e54cf360b9b83f6cd93bd551964bf1..dad116096449948be8613c1ac44884f948569707 100644 (file)
@@ -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;
index a7685efc4bb6eaa5326f7633ef1fcbf64d463725..157d700ea1461c4fdcccc0bbd2a3c1b34533b895 100644 (file)
@@ -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;
 }
 
index 9c7a52737f1d0a50c5240073decb42123cfb1c91..78e54faff926fbed8cee65d005068262d8749ba9 100644 (file)
--- 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);
 
index 3b1766bef39f5dbe0b29313506cf2540c9617742..8b7e8ebbe04fe64d8e4b8cd2f623fd938f234d6f 100644 (file)
--- 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
index 9f11ba6a02850800cc11e13962a4709b4d4d5a25..fbab1df9acfd5e874c487a81bd30405595f01e2f 100644 (file)
--- 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 */