[monitoring-plugins] More refactoring

Lorenz Kästle git at monitoring-plugins.org
Wed Mar 12 16:00:12 CET 2025


 Module: monitoring-plugins
 Branch: master
 Commit: ab2c2f525987dba8d314644d9eeeb48f9ae4d70c
 Author: Lorenz Kästle <12514511+RincewindsHat at users.noreply.github.com>
   Date: Wed Mar 12 13:36:04 2025 +0100
    URL: https://www.monitoring-plugins.org/repositories/monitoring-plugins/commit/?id=ab2c2f52

More refactoring

---

 plugins/check_ntp_peer.c          | 186 ++++++++++++++++++++------------------
 plugins/check_ntp_peer.d/config.h |  13 +++
 2 files changed, 109 insertions(+), 90 deletions(-)

diff --git a/plugins/check_ntp_peer.c b/plugins/check_ntp_peer.c
index fcd06b8e..19e8a11f 100644
--- a/plugins/check_ntp_peer.c
+++ b/plugins/check_ntp_peer.c
@@ -35,6 +35,7 @@
  *
  *****************************************************************************/
 
+#include "thresholds.h"
 const char *progname = "check_ntp_peer";
 const char *copyright = "2006-2024";
 const char *email = "devel at monitoring-plugins.org";
@@ -54,10 +55,6 @@ typedef struct {
 	check_ntp_peer_config config;
 } check_ntp_peer_config_wrapper;
 static check_ntp_peer_config_wrapper process_arguments(int /*argc*/, char ** /*argv*/);
-static thresholds *offset_thresholds = NULL;
-static thresholds *jitter_thresholds = NULL;
-static thresholds *stratum_thresholds = NULL;
-static thresholds *truechimer_thresholds = NULL;
 static void print_help(void);
 void print_usage(void);
 
@@ -149,25 +146,25 @@ typedef struct {
 		printf("%u.%u.%u.%u", (x >> 24) & 0xff, (x >> 16) & 0xff, (x >> 8) & 0xff, x & 0xff);                                              \
 	} while (0);
 
-void print_ntp_control_message(const ntp_control_message *p) {
+void print_ntp_control_message(const ntp_control_message *message) {
 	printf("control packet contents:\n");
-	printf("\tflags: 0x%.2x , 0x%.2x\n", p->flags, p->op);
-	printf("\t  li=%d (0x%.2x)\n", LI(p->flags), p->flags & LI_MASK);
-	printf("\t  vn=%d (0x%.2x)\n", VN(p->flags), p->flags & VN_MASK);
-	printf("\t  mode=%d (0x%.2x)\n", MODE(p->flags), p->flags & MODE_MASK);
-	printf("\t  response=%d (0x%.2x)\n", (p->op & REM_RESP) > 0, p->op & REM_RESP);
-	printf("\t  more=%d (0x%.2x)\n", (p->op & REM_MORE) > 0, p->op & REM_MORE);
-	printf("\t  error=%d (0x%.2x)\n", (p->op & REM_ERROR) > 0, p->op & REM_ERROR);
-	printf("\t  op=%d (0x%.2x)\n", p->op & OP_MASK, p->op & OP_MASK);
-	printf("\tsequence: %d (0x%.2x)\n", ntohs(p->seq), ntohs(p->seq));
-	printf("\tstatus: %d (0x%.2x)\n", ntohs(p->status), ntohs(p->status));
-	printf("\tassoc: %d (0x%.2x)\n", ntohs(p->assoc), ntohs(p->assoc));
-	printf("\toffset: %d (0x%.2x)\n", ntohs(p->offset), ntohs(p->offset));
-	printf("\tcount: %d (0x%.2x)\n", ntohs(p->count), ntohs(p->count));
-
-	int numpeers = ntohs(p->count) / (sizeof(ntp_assoc_status_pair));
-	if (p->op & REM_RESP && p->op & OP_READSTAT) {
-		const ntp_assoc_status_pair *peer = (ntp_assoc_status_pair *)p->data;
+	printf("\tflags: 0x%.2x , 0x%.2x\n", message->flags, message->op);
+	printf("\t  li=%d (0x%.2x)\n", LI(message->flags), message->flags & LI_MASK);
+	printf("\t  vn=%d (0x%.2x)\n", VN(message->flags), message->flags & VN_MASK);
+	printf("\t  mode=%d (0x%.2x)\n", MODE(message->flags), message->flags & MODE_MASK);
+	printf("\t  response=%d (0x%.2x)\n", (message->op & REM_RESP) > 0, message->op & REM_RESP);
+	printf("\t  more=%d (0x%.2x)\n", (message->op & REM_MORE) > 0, message->op & REM_MORE);
+	printf("\t  error=%d (0x%.2x)\n", (message->op & REM_ERROR) > 0, message->op & REM_ERROR);
+	printf("\t  op=%d (0x%.2x)\n", message->op & OP_MASK, message->op & OP_MASK);
+	printf("\tsequence: %d (0x%.2x)\n", ntohs(message->seq), ntohs(message->seq));
+	printf("\tstatus: %d (0x%.2x)\n", ntohs(message->status), ntohs(message->status));
+	printf("\tassoc: %d (0x%.2x)\n", ntohs(message->assoc), ntohs(message->assoc));
+	printf("\toffset: %d (0x%.2x)\n", ntohs(message->offset), ntohs(message->offset));
+	printf("\tcount: %d (0x%.2x)\n", ntohs(message->count), ntohs(message->count));
+
+	int numpeers = ntohs(message->count) / (sizeof(ntp_assoc_status_pair));
+	if (message->op & REM_RESP && message->op & OP_READSTAT) {
+		const ntp_assoc_status_pair *peer = (ntp_assoc_status_pair *)message->data;
 		for (int i = 0; i < numpeers; i++) {
 			printf("\tpeer id %.2x status %.2x", ntohs(peer[i].assoc), ntohs(peer[i].status));
 			if (PEER_SEL(peer[i].status) >= PEER_SYNCSOURCE) {
@@ -182,13 +179,13 @@ void print_ntp_control_message(const ntp_control_message *p) {
 	}
 }
 
-void setup_control_request(ntp_control_message *p, uint8_t opcode, uint16_t seq) {
-	memset(p, 0, sizeof(ntp_control_message));
-	LI_SET(p->flags, LI_NOWARNING);
-	VN_SET(p->flags, VN_RESERVED);
-	MODE_SET(p->flags, MODE_CONTROLMSG);
-	OP_SET(p->op, opcode);
-	p->seq = htons(seq);
+void setup_control_request(ntp_control_message *message, uint8_t opcode, uint16_t seq) {
+	memset(message, 0, sizeof(ntp_control_message));
+	LI_SET(message->flags, LI_NOWARNING);
+	VN_SET(message->flags, VN_RESERVED);
+	MODE_SET(message->flags, MODE_CONTROLMSG);
+	OP_SET(message->op, opcode);
+	message->seq = htons(seq);
 	/* Remaining fields are zero for requests */
 }
 
@@ -203,11 +200,23 @@ void setup_control_request(ntp_control_message *p, uint8_t opcode, uint16_t seq)
  *  status is pretty much useless as syncsource_found is a global variable
  *  used later in main to check is the server was synchronized. It works
  *  so I left it alone */
-mp_state_enum ntp_request(double *offset, int *offset_result, double *jitter, int *stratum, int *num_truechimers,
-						  const check_ntp_peer_config config) {
-	*offset_result = STATE_UNKNOWN;
-	*jitter = *stratum = -1;
-	*num_truechimers = 0;
+typedef struct {
+	mp_state_enum state;
+	mp_state_enum offset_result;
+	double offset;
+	double jitter;
+	long stratum;
+	int num_truechimers;
+} ntp_request_result;
+ntp_request_result ntp_request(const check_ntp_peer_config config) {
+
+	ntp_request_result result = {
+		.state = STATE_OK,
+		.offset_result = STATE_UNKNOWN,
+		.jitter = -1,
+		.stratum = -1,
+		.num_truechimers = 0,
+	};
 
 	/* Long-winded explanation:
 	 * Getting the sync peer offset, jitter and stratum requires a number of
@@ -230,8 +239,8 @@ mp_state_enum ntp_request(double *offset, int *offset_result, double *jitter, in
 	void *tmp;
 	ntp_assoc_status_pair *peers = NULL;
 	int peer_offset = 0;
-	int peers_size = 0;
-	int npeers = 0;
+	size_t peers_size = 0;
+	size_t npeers = 0;
 	int conn = -1;
 	my_udp_connect(config.server_address, config.port, &conn);
 
@@ -269,7 +278,7 @@ mp_state_enum ntp_request(double *offset, int *offset_result, double *jitter, in
 			free(peers), die(STATE_UNKNOWN, "can not (re)allocate 'peers' buffer\n");
 		}
 		peers = tmp;
-		memcpy((void *)((ptrdiff_t)peers + peer_offset), (void *)req.data, ntohs(req.count));
+		memcpy((peers + peer_offset), (void *)req.data, ntohs(req.count));
 		npeers = peers_size / sizeof(ntp_assoc_status_pair);
 		peer_offset += ntohs(req.count);
 	} while (req.op & REM_MORE);
@@ -277,9 +286,9 @@ mp_state_enum ntp_request(double *offset, int *offset_result, double *jitter, in
 	/* first, let's find out if we have a sync source, or if there are
 	 * at least some candidates. In the latter case we'll issue
 	 * a warning but go ahead with the check on them. */
-	for (int i = 0; i < npeers; i++) {
+	for (size_t i = 0; i < npeers; i++) {
 		if (PEER_SEL(peers[i].status) >= PEER_TRUECHIMER) {
-			(*num_truechimers)++;
+			result.num_truechimers++;
 			if (PEER_SEL(peers[i].status) >= PEER_INCLUDED) {
 				num_candidates++;
 				if (PEER_SEL(peers[i].status) >= PEER_SYNCSOURCE) {
@@ -297,15 +306,14 @@ mp_state_enum ntp_request(double *offset, int *offset_result, double *jitter, in
 		printf("synchronization source found\n");
 	}
 
-	int status = STATE_OK;
 	if (!syncsource_found) {
-		status = STATE_WARNING;
+		result.state = STATE_WARNING;
 		if (verbose) {
 			printf("warning: no synchronization source found\n");
 		}
 	}
 	if (li_alarm) {
-		status = STATE_WARNING;
+		result.state = STATE_WARNING;
 		if (verbose) {
 			printf("warning: LI_ALARM bit is set\n");
 		}
@@ -313,7 +321,7 @@ mp_state_enum ntp_request(double *offset, int *offset_result, double *jitter, in
 
 	const char *getvar = "stratum,offset,jitter";
 	char *data;
-	for (int i = 0; i < npeers; i++) {
+	for (size_t i = 0; i < npeers; i++) {
 		/* Only query this server if it is the current sync source */
 		/* If there's no sync.peer, query all candidates and use the best one */
 		if (PEER_SEL(peers[i].status) >= min_peer_sel) {
@@ -396,9 +404,9 @@ mp_state_enum ntp_request(double *offset, int *offset_result, double *jitter, in
 				if (verbose) {
 					printf("%.10g\n", tmp_offset);
 				}
-				if (*offset_result == STATE_UNKNOWN || fabs(tmp_offset) < fabs(*offset)) {
-					*offset = tmp_offset;
-					*offset_result = STATE_OK;
+				if (result.offset_result == STATE_UNKNOWN || fabs(tmp_offset) < fabs(result.offset)) {
+					result.offset = tmp_offset;
+					result.offset_result = STATE_OK;
 				} else {
 					/* Skip this one; move to the next */
 					continue;
@@ -415,16 +423,16 @@ mp_state_enum ntp_request(double *offset, int *offset_result, double *jitter, in
 				nptr = NULL;
 				/* Convert the value if we have one */
 				if (value != NULL) {
-					*jitter = strtod(value, &nptr);
+					result.jitter = strtod(value, &nptr);
 				}
 				/* If value is null or no conversion was performed */
 				if (value == NULL || value == nptr) {
 					if (verbose) {
 						printf("error: unable to read server jitter/dispersion response.\n");
 					}
-					*jitter = -1;
+					result.jitter = -1;
 				} else if (verbose) {
-					printf("%.10g\n", *jitter);
+					printf("%.10g\n", result.jitter);
 				}
 			}
 
@@ -437,16 +445,16 @@ mp_state_enum ntp_request(double *offset, int *offset_result, double *jitter, in
 				nptr = NULL;
 				/* Convert the value if we have one */
 				if (value != NULL) {
-					*stratum = strtol(value, &nptr, 10);
+					result.stratum = strtol(value, &nptr, 10);
 				}
 				if (value == NULL || value == nptr) {
 					if (verbose) {
 						printf("error: unable to read server stratum response.\n");
 					}
-					*stratum = -1;
+					result.stratum = -1;
 				} else {
 					if (verbose) {
-						printf("%i\n", *stratum);
+						printf("%li\n", result.stratum);
 					}
 				}
 			}
@@ -458,7 +466,7 @@ mp_state_enum ntp_request(double *offset, int *offset_result, double *jitter, in
 		free(peers);
 	}
 
-	return status;
+	return result;
 }
 
 check_ntp_peer_config_wrapper process_arguments(int argc, char **argv) {
@@ -564,25 +572,30 @@ check_ntp_peer_config_wrapper process_arguments(int argc, char **argv) {
 		usage4(_("Hostname was not supplied"));
 	}
 
+	set_thresholds(&result.config.offset_thresholds, result.config.owarn, result.config.ocrit);
+	set_thresholds(&result.config.jitter_thresholds, result.config.jwarn, result.config.jcrit);
+	set_thresholds(&result.config.stratum_thresholds, result.config.swarn, result.config.scrit);
+	set_thresholds(&result.config.truechimer_thresholds, result.config.twarn, result.config.tcrit);
+
 	return result;
 }
 
-char *perfd_offset(double offset) {
+char *perfd_offset(double offset, thresholds *offset_thresholds) {
 	return fperfdata("offset", offset, "s", true, offset_thresholds->warning->end, true, offset_thresholds->critical->end, false, 0, false,
 					 0);
 }
 
-char *perfd_jitter(double jitter, bool do_jitter) {
+char *perfd_jitter(double jitter, bool do_jitter, thresholds *jitter_thresholds) {
 	return fperfdata("jitter", jitter, "", do_jitter, jitter_thresholds->warning->end, do_jitter, jitter_thresholds->critical->end, true, 0,
 					 false, 0);
 }
 
-char *perfd_stratum(int stratum, bool do_stratum) {
+char *perfd_stratum(int stratum, bool do_stratum, thresholds *stratum_thresholds) {
 	return perfdata("stratum", stratum, "", do_stratum, (int)stratum_thresholds->warning->end, do_stratum,
 					(int)stratum_thresholds->critical->end, true, 0, true, 16);
 }
 
-char *perfd_truechimers(int num_truechimers, const bool do_truechimers) {
+char *perfd_truechimers(int num_truechimers, const bool do_truechimers, thresholds *truechimer_thresholds) {
 	return perfdata("truechimers", num_truechimers, "", do_truechimers, (int)truechimer_thresholds->warning->end, do_truechimers,
 					(int)truechimer_thresholds->critical->end, true, 0, false, 0);
 }
@@ -603,26 +616,17 @@ int main(int argc, char *argv[]) {
 
 	const check_ntp_peer_config config = tmp_config.config;
 
-	set_thresholds(&offset_thresholds, config.owarn, config.ocrit);
-	set_thresholds(&jitter_thresholds, config.jwarn, config.jcrit);
-	set_thresholds(&stratum_thresholds, config.swarn, config.scrit);
-	set_thresholds(&truechimer_thresholds, config.twarn, config.tcrit);
-
 	/* initialize alarm signal handling */
 	signal(SIGALRM, socket_timeout_alarm_handler);
 
 	/* set socket timeout */
 	alarm(socket_timeout);
 
-	int offset_result;
-	int stratum;
-	int num_truechimers;
-	double offset = 0;
-	double jitter = 0;
 	/* This returns either OK or WARNING (See comment preceding ntp_request) */
-	mp_state_enum result = ntp_request(&offset, &offset_result, &jitter, &stratum, &num_truechimers, config);
+	ntp_request_result ntp_res = ntp_request(config);
+	mp_state_enum result = STATE_UNKNOWN;
 
-	if (offset_result == STATE_UNKNOWN) {
+	if (ntp_res.offset_result == STATE_UNKNOWN) {
 		/* if there's no sync peer (this overrides ntp_request output): */
 		result = (config.quiet ? STATE_UNKNOWN : STATE_CRITICAL);
 	} else {
@@ -630,28 +634,28 @@ int main(int argc, char *argv[]) {
 		if (config.quiet && result == STATE_WARNING) {
 			result = STATE_UNKNOWN;
 		}
-		result = max_state_alt(result, get_status(fabs(offset), offset_thresholds));
+		result = max_state_alt(result, get_status(fabs(ntp_res.offset), config.offset_thresholds));
 	}
 
 	mp_state_enum oresult = result;
 	mp_state_enum tresult = STATE_UNKNOWN;
 
 	if (config.do_truechimers) {
-		tresult = get_status(num_truechimers, truechimer_thresholds);
+		tresult = get_status(ntp_res.num_truechimers, config.truechimer_thresholds);
 		result = max_state_alt(result, tresult);
 	}
 
 	mp_state_enum sresult = STATE_UNKNOWN;
 
 	if (config.do_stratum) {
-		sresult = get_status(stratum, stratum_thresholds);
+		sresult = get_status((double)ntp_res.stratum, config.stratum_thresholds);
 		result = max_state_alt(result, sresult);
 	}
 
 	mp_state_enum jresult = STATE_UNKNOWN;
 
 	if (config.do_jitter) {
-		jresult = get_status(jitter, jitter_thresholds);
+		jresult = get_status(ntp_res.jitter, config.jitter_thresholds);
 		result = max_state_alt(result, jresult);
 	}
 
@@ -678,50 +682,52 @@ int main(int argc, char *argv[]) {
 	}
 
 	char *perfdata_line;
-	if (offset_result == STATE_UNKNOWN) {
+	if (ntp_res.offset_result == STATE_UNKNOWN) {
 		xasprintf(&result_line, "%s %s", result_line, _("Offset unknown"));
 		xasprintf(&perfdata_line, "");
 	} else if (oresult == STATE_WARNING) {
-		xasprintf(&result_line, "%s %s %.10g secs (WARNING)", result_line, _("Offset"), offset);
+		xasprintf(&result_line, "%s %s %.10g secs (WARNING)", result_line, _("Offset"), ntp_res.offset);
 	} else if (oresult == STATE_CRITICAL) {
-		xasprintf(&result_line, "%s %s %.10g secs (CRITICAL)", result_line, _("Offset"), offset);
+		xasprintf(&result_line, "%s %s %.10g secs (CRITICAL)", result_line, _("Offset"), ntp_res.offset);
 	} else {
-		xasprintf(&result_line, "%s %s %.10g secs", result_line, _("Offset"), offset);
+		xasprintf(&result_line, "%s %s %.10g secs", result_line, _("Offset"), ntp_res.offset);
 	}
-	xasprintf(&perfdata_line, "%s", perfd_offset(offset));
+	xasprintf(&perfdata_line, "%s", perfd_offset(ntp_res.offset, config.offset_thresholds));
 
 	if (config.do_jitter) {
 		if (jresult == STATE_WARNING) {
-			xasprintf(&result_line, "%s, jitter=%f (WARNING)", result_line, jitter);
+			xasprintf(&result_line, "%s, jitter=%f (WARNING)", result_line, ntp_res.jitter);
 		} else if (jresult == STATE_CRITICAL) {
-			xasprintf(&result_line, "%s, jitter=%f (CRITICAL)", result_line, jitter);
+			xasprintf(&result_line, "%s, jitter=%f (CRITICAL)", result_line, ntp_res.jitter);
 		} else {
-			xasprintf(&result_line, "%s, jitter=%f", result_line, jitter);
+			xasprintf(&result_line, "%s, jitter=%f", result_line, ntp_res.jitter);
 		}
-		xasprintf(&perfdata_line, "%s %s", perfdata_line, perfd_jitter(jitter, config.do_jitter));
+		xasprintf(&perfdata_line, "%s %s", perfdata_line, perfd_jitter(ntp_res.jitter, config.do_jitter, config.jitter_thresholds));
 	}
 
 	if (config.do_stratum) {
 		if (sresult == STATE_WARNING) {
-			xasprintf(&result_line, "%s, stratum=%i (WARNING)", result_line, stratum);
+			xasprintf(&result_line, "%s, stratum=%i (WARNING)", result_line, ntp_res.stratum);
 		} else if (sresult == STATE_CRITICAL) {
-			xasprintf(&result_line, "%s, stratum=%i (CRITICAL)", result_line, stratum);
+			xasprintf(&result_line, "%s, stratum=%i (CRITICAL)", result_line, ntp_res.stratum);
 		} else {
-			xasprintf(&result_line, "%s, stratum=%i", result_line, stratum);
+			xasprintf(&result_line, "%s, stratum=%i", result_line, ntp_res.stratum);
 		}
-		xasprintf(&perfdata_line, "%s %s", perfdata_line, perfd_stratum(stratum, config.do_stratum));
+		xasprintf(&perfdata_line, "%s %s", perfdata_line, perfd_stratum(ntp_res.stratum, config.do_stratum, config.stratum_thresholds));
 	}
 
 	if (config.do_truechimers) {
 		if (tresult == STATE_WARNING) {
-			xasprintf(&result_line, "%s, truechimers=%i (WARNING)", result_line, num_truechimers);
+			xasprintf(&result_line, "%s, truechimers=%i (WARNING)", result_line, ntp_res.num_truechimers);
 		} else if (tresult == STATE_CRITICAL) {
-			xasprintf(&result_line, "%s, truechimers=%i (CRITICAL)", result_line, num_truechimers);
+			xasprintf(&result_line, "%s, truechimers=%i (CRITICAL)", result_line, ntp_res.num_truechimers);
 		} else {
-			xasprintf(&result_line, "%s, truechimers=%i", result_line, num_truechimers);
+			xasprintf(&result_line, "%s, truechimers=%i", result_line, ntp_res.num_truechimers);
 		}
-		xasprintf(&perfdata_line, "%s %s", perfdata_line, perfd_truechimers(num_truechimers, config.do_truechimers));
+		xasprintf(&perfdata_line, "%s %s", perfdata_line,
+				  perfd_truechimers(ntp_res.num_truechimers, config.do_truechimers, config.truechimer_thresholds));
 	}
+
 	printf("%s|%s\n", result_line, perfdata_line);
 
 	if (config.server_address != NULL) {
diff --git a/plugins/check_ntp_peer.d/config.h b/plugins/check_ntp_peer.d/config.h
index 1907af7c..00e6b05d 100644
--- a/plugins/check_ntp_peer.d/config.h
+++ b/plugins/check_ntp_peer.d/config.h
@@ -1,6 +1,7 @@
 #pragma once
 
 #include "../../config.h"
+#include "thresholds.h"
 #include <stddef.h>
 
 enum {
@@ -17,19 +18,24 @@ typedef struct {
 	bool do_truechimers;
 	char *twarn;
 	char *tcrit;
+	thresholds *truechimer_thresholds;
 
 	char *owarn;
 	char *ocrit;
+	thresholds *offset_thresholds;
 
 	// stratum stuff
 	bool do_stratum;
 	char *swarn;
 	char *scrit;
+	thresholds *stratum_thresholds;
 
 	// jitter stuff
 	bool do_jitter;
 	char *jwarn;
 	char *jcrit;
+	thresholds *jitter_thresholds;
+
 } check_ntp_peer_config;
 
 check_ntp_peer_config check_ntp_peer_config_init() {
@@ -41,14 +47,21 @@ check_ntp_peer_config check_ntp_peer_config_init() {
 		.do_truechimers = false,
 		.twarn = "0:",
 		.tcrit = "0:",
+		.truechimer_thresholds = NULL,
+
 		.owarn = "60",
 		.ocrit = "120",
+		.offset_thresholds = NULL,
+
 		.do_stratum = false,
 		.swarn = "-1:16",
 		.scrit = "-1:16",
+		.stratum_thresholds = NULL,
+
 		.do_jitter = false,
 		.jwarn = "-1:5000",
 		.jcrit = "-1:10000",
+		.jitter_thresholds = NULL,
 	};
 	return tmp;
 }



More information about the Commits mailing list