Module: monitoring-plugins
 Branch: master
 Commit: cf48162487d6f042af084ee436acdd7a2db0cbfd
 Author: Lorenz Kästle <12514511+rincewinds...@users.noreply.github.com>
   Date: Tue Mar 11 02:43:51 2025 +0100
    URL: 
https://www.monitoring-plugins.org/repositories/monitoring-plugins/commit/?id=cf481624

Refactor check_fping

---

 plugins/Makefile.am            |   3 +-
 plugins/check_fping.c          | 182 ++++++++++++++++++++---------------------
 plugins/check_fping.d/config.h |  58 +++++++++++++
 3 files changed, 147 insertions(+), 96 deletions(-)

diff --git a/plugins/Makefile.am b/plugins/Makefile.am
index 41487131..25a286c1 100644
--- a/plugins/Makefile.am
+++ b/plugins/Makefile.am
@@ -57,7 +57,8 @@ EXTRA_DIST = t \
                         check_apt.d \
                         check_by_ssh.d \
                         check_smtp.d \
-                        check_dig.d
+                        check_dig.d \
+                        check_fping.d
 
 PLUGINHDRS = common.h
 
diff --git a/plugins/check_fping.c b/plugins/check_fping.c
index b85397d5..ec7abb67 100644
--- a/plugins/check_fping.c
+++ b/plugins/check_fping.c
@@ -38,52 +38,29 @@ const char *email = "devel@monitoring-plugins.org";
 #include "netutils.h"
 #include "utils.h"
 #include <stdbool.h>
+#include "check_fping.d/config.h"
+#include "states.h"
 
 enum {
-       PACKET_COUNT = 1,
-       PACKET_SIZE = 56,
        PL = 0,
        RTA = 1
 };
 
-static int textscan(char *buf);
-static int process_arguments(int /*argc*/, char ** /*argv*/);
+static mp_state_enum textscan(char *buf, const char * /*server_name*/, bool 
/*crta_p*/, double /*crta*/, bool /*wrta_p*/, double /*wrta*/,
+                                                         bool /*cpl_p*/, int 
/*cpl*/, bool /*wpl_p*/, int /*wpl*/, bool /*alive_p*/);
+
+typedef struct {
+       int errorcode;
+       check_fping_config config;
+} check_fping_config_wrapper;
+static check_fping_config_wrapper process_arguments(int /*argc*/, char ** 
/*argv*/);
 static int get_threshold(char *arg, char *rv[2]);
 static void print_help(void);
 void print_usage(void);
 
-static char *server_name = NULL;
-static char *sourceip = NULL;
-static char *sourceif = NULL;
-static int packet_size = PACKET_SIZE;
-static int packet_count = PACKET_COUNT;
-static int target_timeout = 0;
-static int packet_interval = 0;
 static bool verbose = false;
-static bool dontfrag = false;
-static bool randomize_packet_data = false;
-static int cpl;
-static int wpl;
-static double crta;
-static double wrta;
-static bool cpl_p = false;
-static bool wpl_p = false;
-static bool alive_p = false;
-static bool crta_p = false;
-static bool wrta_p = false;
 
 int main(int argc, char **argv) {
-       /* normally should be  int result = STATE_UNKNOWN; */
-
-       int status = STATE_UNKNOWN;
-       int result = 0;
-       char *fping_prog = NULL;
-       char *server = NULL;
-       char *command_line = NULL;
-       char *input_buffer = NULL;
-       char *option_string = "";
-       input_buffer = malloc(MAX_INPUT_BUFFER);
-
        setlocale(LC_ALL, "");
        bindtextdomain(PACKAGE, LOCALEDIR);
        textdomain(PACKAGE);
@@ -91,32 +68,38 @@ int main(int argc, char **argv) {
        /* Parse extra opts if any */
        argv = np_extra_opts(&argc, argv, progname);
 
-       if (process_arguments(argc, argv) == ERROR) {
+       check_fping_config_wrapper tmp_config = process_arguments(argc, argv);
+       if (tmp_config.errorcode == ERROR) {
                usage4(_("Could not parse arguments"));
        }
 
-       server = strscpy(server, server_name);
+       const check_fping_config config = tmp_config.config;
+
+       char *server = NULL;
+       server = strscpy(server, config.server_name);
 
+       char *option_string = "";
        /* compose the command */
-       if (target_timeout) {
-               xasprintf(&option_string, "%s-t %d ", option_string, 
target_timeout);
+       if (config.target_timeout) {
+               xasprintf(&option_string, "%s-t %d ", option_string, 
config.target_timeout);
        }
-       if (packet_interval) {
-               xasprintf(&option_string, "%s-p %d ", option_string, 
packet_interval);
+       if (config.packet_interval) {
+               xasprintf(&option_string, "%s-p %d ", option_string, 
config.packet_interval);
        }
-       if (sourceip) {
-               xasprintf(&option_string, "%s-S %s ", option_string, sourceip);
+       if (config.sourceip) {
+               xasprintf(&option_string, "%s-S %s ", option_string, 
config.sourceip);
        }
-       if (sourceif) {
-               xasprintf(&option_string, "%s-I %s ", option_string, sourceif);
+       if (config.sourceif) {
+               xasprintf(&option_string, "%s-I %s ", option_string, 
config.sourceif);
        }
-       if (dontfrag) {
+       if (config.dontfrag) {
                xasprintf(&option_string, "%s-M ", option_string);
        }
-       if (randomize_packet_data) {
+       if (config.randomize_packet_data) {
                xasprintf(&option_string, "%s-R ", option_string);
        }
 
+       char *fping_prog = NULL;
 #ifdef PATH_TO_FPING6
        if (address_family != AF_INET && is_inet6_addr(server)) {
                fping_prog = strdup(PATH_TO_FPING6);
@@ -127,7 +110,8 @@ int main(int argc, char **argv) {
        fping_prog = strdup(PATH_TO_FPING);
 #endif
 
-       xasprintf(&command_line, "%s %s-b %d -c %d %s", fping_prog, 
option_string, packet_size, packet_count, server);
+       char *command_line = NULL;
+       xasprintf(&command_line, "%s %s-b %d -c %d %s", fping_prog, 
option_string, config.packet_size, config.packet_count, server);
 
        if (verbose) {
                printf("%s\n", command_line);
@@ -145,11 +129,14 @@ int main(int argc, char **argv) {
                printf(_("Could not open stderr for %s\n"), command_line);
        }
 
+       char *input_buffer = malloc(MAX_INPUT_BUFFER);
+       mp_state_enum status = STATE_UNKNOWN;
        while (fgets(input_buffer, MAX_INPUT_BUFFER - 1, child_process)) {
                if (verbose) {
                        printf("%s", input_buffer);
                }
-               status = max_state(status, textscan(input_buffer));
+               status = max_state(status, textscan(input_buffer, 
config.server_name, config.crta_p, config.crta, config.wrta_p, config.wrta,
+                                                                               
        config.cpl_p, config.cpl, config.wpl_p, config.wpl, config.alive_p));
        }
 
        /* If we get anything on STDERR, at least set warning */
@@ -158,12 +145,13 @@ int main(int argc, char **argv) {
                if (verbose) {
                        printf("%s", input_buffer);
                }
-               status = max_state(status, textscan(input_buffer));
+               status = max_state(status, textscan(input_buffer, 
config.server_name, config.crta_p, config.crta, config.wrta_p, config.wrta,
+                                                                               
        config.cpl_p, config.cpl, config.wpl_p, config.wpl, config.alive_p));
        }
        (void)fclose(child_stderr);
 
        /* close the pipe */
-       result = spclose(child_process);
+       int result = spclose(child_process);
        if (result) {
                /* need to use max_state not max */
                status = max_state(status, STATE_WARNING);
@@ -182,21 +170,17 @@ int main(int argc, char **argv) {
                }
        }
 
-       printf("FPING %s - %s\n", state_text(status), server_name);
+       printf("FPING %s - %s\n", state_text(status), config.server_name);
 
        return status;
 }
 
-int textscan(char *buf) {
-       char *rtastr = NULL;
-       char *losstr = NULL;
-       char *xmtstr = NULL;
-       double loss;
-       double rta;
-       double xmt;
-       int status = STATE_UNKNOWN;
-
+mp_state_enum textscan(char *buf, const char *server_name, bool crta_p, double 
crta, bool wrta_p, double wrta, bool cpl_p, int cpl,
+                                          bool wpl_p, int wpl, bool alive_p) {
        /* stops testing after the first successful reply. */
+       double rta;
+       double loss;
+       char *rtastr = NULL;
        if (alive_p && strstr(buf, "avg, 0% loss)")) {
                rtastr = strstr(buf, "ms (");
                rtastr = 1 + index(rtastr, '(');
@@ -208,6 +192,10 @@ int textscan(char *buf) {
                        fperfdata("rta", rta / 1.0e3, "s", wrta_p, wrta / 
1.0e3, crta_p, crta / 1.0e3, true, 0, false, 0));
        }
 
+       mp_state_enum status = STATE_UNKNOWN;
+       char *xmtstr = NULL;
+       double xmt;
+       char *losstr = NULL;
        if (strstr(buf, "not found")) {
                die(STATE_CRITICAL, _("FPING UNKNOWN - %s not found\n"), 
server_name);
 
@@ -243,7 +231,7 @@ int textscan(char *buf) {
                        status = STATE_OK;
                }
                die(status, _("FPING %s - %s (loss=%.0f%%, rta=%f ms)|%s 
%s\n"), state_text(status), server_name, loss, rta,
-                       perfdata("loss", (long int)loss, "%", wpl_p, wpl, 
cpl_p, cpl, true, 0, true, 100),
+                       perfdata("loss", (long int)loss, "%", wpl_p, wpl, 
cpl_p, cpl, false, 0, false, 0),
                        fperfdata("rta", rta / 1.0e3, "s", wrta_p, wrta / 
1.0e3, crta_p, crta / 1.0e3, true, 0, false, 0));
 
        } else if (strstr(buf, "xmt/rcv/%loss")) {
@@ -269,7 +257,7 @@ int textscan(char *buf) {
                }
                /* loss=%.0f%%;%d;%d;0;100 */
                die(status, _("FPING %s - %s (loss=%.0f%% )|%s\n"), 
state_text(status), server_name, loss,
-                       perfdata("loss", (long int)loss, "%", wpl_p, wpl, 
cpl_p, cpl, true, 0, true, 100));
+                       perfdata("loss", (long int)loss, "%", wpl_p, wpl, 
cpl_p, cpl, false, 0, false, 0));
 
        } else {
                status = max_state(status, STATE_WARNING);
@@ -279,11 +267,7 @@ int textscan(char *buf) {
 }
 
 /* process command-line arguments */
-int process_arguments(int argc, char **argv) {
-       int c;
-       char *rv[2];
-
-       int option = 0;
+check_fping_config_wrapper process_arguments(int argc, char **argv) {
        static struct option longopts[] = {
                {"hostname", required_argument, 0, 'H'}, {"sourceip", 
required_argument, 0, 'S'}, {"sourceif", required_argument, 0, 'I'},
                {"critical", required_argument, 0, 'c'}, {"warning", 
required_argument, 0, 'w'},  {"alive", no_argument, 0, 'a'},
@@ -292,32 +276,41 @@ int process_arguments(int argc, char **argv) {
                {"help", no_argument, 0, 'h'},           {"use-ipv4", 
no_argument, 0, '4'},       {"use-ipv6", no_argument, 0, '6'},
                {"dontfrag", no_argument, 0, 'M'},       {"random", 
no_argument, 0, 'R'},         {0, 0, 0, 0}};
 
+       char *rv[2];
        rv[PL] = NULL;
        rv[RTA] = NULL;
 
+       int option = 0;
+
+       check_fping_config_wrapper result = {
+               .errorcode = OK,
+               .config = check_fping_config_init(),
+       };
+
        if (argc < 2) {
-               return ERROR;
+               result.errorcode = ERROR;
+               return result;
        }
 
        if (!is_option(argv[1])) {
-               server_name = argv[1];
+               result.config.server_name = argv[1];
                argv[1] = argv[0];
                argv = &argv[1];
                argc--;
        }
 
        while (1) {
-               c = getopt_long(argc, argv, "+hVvaH:S:c:w:b:n:T:i:I:M:R:46", 
longopts, &option);
+               int option_index = getopt_long(argc, argv, 
"+hVvaH:S:c:w:b:n:T:i:I:M:R:46", longopts, &option);
 
-               if (c == -1 || c == EOF || c == 1) {
+               if (option_index == -1 || option_index == EOF || option_index 
== 1) {
                        break;
                }
 
-               switch (c) {
+               switch (option_index) {
                case '?': /* print short usage statement if args not parsable */
                        usage5();
                case 'a': /* host alive mode */
-                       alive_p = true;
+                       result.config.alive_p = true;
                        break;
                case 'h': /* help */
                        print_help();
@@ -329,19 +322,19 @@ int process_arguments(int argc, char **argv) {
                        verbose = true;
                        break;
                case 'H': /* hostname */
-                       if (is_host(optarg) == false) {
+                       if (!is_host(optarg)) {
                                usage2(_("Invalid hostname/address"), optarg);
                        }
-                       server_name = strscpy(server_name, optarg);
+                       result.config.server_name = optarg;
                        break;
                case 'S': /* sourceip */
-                       if (is_host(optarg) == false) {
+                       if (!is_host(optarg)) {
                                usage2(_("Invalid hostname/address"), optarg);
                        }
-                       sourceip = strscpy(sourceip, optarg);
+                       result.config.sourceip = optarg;
                        break;
                case 'I': /* sourceip */
-                       sourceif = strscpy(sourceif, optarg);
+                       result.config.sourceif = optarg;
                        break;
                case '4': /* IPv4 only */
                        address_family = AF_INET;
@@ -356,78 +349,77 @@ int process_arguments(int argc, char **argv) {
                case 'c':
                        get_threshold(optarg, rv);
                        if (rv[RTA]) {
-                               crta = strtod(rv[RTA], NULL);
-                               crta_p = true;
+                               result.config.crta = strtod(rv[RTA], NULL);
+                               result.config.crta_p = true;
                                rv[RTA] = NULL;
                        }
                        if (rv[PL]) {
-                               cpl = atoi(rv[PL]);
-                               cpl_p = true;
+                               result.config.cpl = atoi(rv[PL]);
+                               result.config.cpl_p = true;
                                rv[PL] = NULL;
                        }
                        break;
                case 'w':
                        get_threshold(optarg, rv);
                        if (rv[RTA]) {
-                               wrta = strtod(rv[RTA], NULL);
-                               wrta_p = true;
+                               result.config.wrta = strtod(rv[RTA], NULL);
+                               result.config.wrta_p = true;
                                rv[RTA] = NULL;
                        }
                        if (rv[PL]) {
-                               wpl = atoi(rv[PL]);
-                               wpl_p = true;
+                               result.config.wpl = atoi(rv[PL]);
+                               result.config.wpl_p = true;
                                rv[PL] = NULL;
                        }
                        break;
                case 'b': /* bytes per packet */
                        if (is_intpos(optarg)) {
-                               packet_size = atoi(optarg);
+                               result.config.packet_size = atoi(optarg);
                        } else {
                                usage(_("Packet size must be a positive 
integer"));
                        }
                        break;
                case 'n': /* number of packets */
                        if (is_intpos(optarg)) {
-                               packet_count = atoi(optarg);
+                               result.config.packet_count = atoi(optarg);
                        } else {
                                usage(_("Packet count must be a positive 
integer"));
                        }
                        break;
                case 'T': /* timeout in msec */
                        if (is_intpos(optarg)) {
-                               target_timeout = atoi(optarg);
+                               result.config.target_timeout = atoi(optarg);
                        } else {
                                usage(_("Target timeout must be a positive 
integer"));
                        }
                        break;
                case 'i': /* interval in msec */
                        if (is_intpos(optarg)) {
-                               packet_interval = atoi(optarg);
+                               result.config.packet_interval = atoi(optarg);
                        } else {
                                usage(_("Interval must be a positive integer"));
                        }
                        break;
                case 'R':
-                       randomize_packet_data = true;
+                       result.config.randomize_packet_data = true;
                        break;
                case 'M':
-                       dontfrag = true;
+                       result.config.dontfrag = true;
                        break;
                }
        }
 
-       if (server_name == NULL) {
+       if (result.config.server_name == NULL) {
                usage4(_("Hostname was not supplied"));
        }
 
-       return OK;
+       return result;
 }
 
 int get_threshold(char *arg, char *rv[2]) {
-       char *arg1 = NULL;
        char *arg2 = NULL;
 
-       arg1 = strscpy(arg1, arg);
+       char *arg1 = strdup(arg);
        if (strpbrk(arg1, ",:")) {
                arg2 = 1 + strpbrk(arg1, ",:");
        }
diff --git a/plugins/check_fping.d/config.h b/plugins/check_fping.d/config.h
new file mode 100644
index 00000000..a0697bf3
--- /dev/null
+++ b/plugins/check_fping.d/config.h
@@ -0,0 +1,58 @@
+#pragma once
+
+#include "../../config.h"
+#include <stddef.h>
+
+enum {
+       PACKET_SIZE = 56,
+       PACKET_COUNT = 1,
+};
+
+typedef struct {
+       char *server_name;
+       char *sourceip;
+       char *sourceif;
+       int packet_size;
+       int packet_count;
+       int target_timeout;
+       int packet_interval;
+       bool randomize_packet_data;
+       bool dontfrag;
+       bool alive_p;
+
+       double crta;
+       bool crta_p;
+       double wrta;
+       bool wrta_p;
+
+       int cpl;
+       bool cpl_p;
+       int wpl;
+       bool wpl_p;
+} check_fping_config;
+
+check_fping_config check_fping_config_init() {
+       check_fping_config tmp = {
+               .server_name = NULL,
+               .sourceip = NULL,
+               .sourceif = NULL,
+               .packet_size = PACKET_SIZE,
+               .packet_count = PACKET_COUNT,
+               .target_timeout = 0,
+               .packet_interval = 0,
+               .randomize_packet_data = false,
+               .dontfrag = false,
+               .alive_p = false,
+
+               .crta = 0,
+               .crta_p = false,
+               .wrta = 0,
+               .wrta_p = false,
+
+               .cpl = 0,
+               .cpl_p = false,
+               .wpl = 0,
+               .wpl_p = false,
+       };
+       return tmp;
+}

Reply via email to