From dd6cdbe89c7c82d3927d68850e0286c8130ce77f Mon Sep 17 00:00:00 2001 From: Arnaud Quette Date: Wed, 13 Mar 2019 16:32:17 +0100 Subject: [PATCH 01/10] Use str* API to check for valid numeric value Closes networkupstools/nut#676 Signed-off-by: Arnaud Quette --- server/conf.c | 42 ++++++++++++++++++------------------------ 1 file changed, 18 insertions(+), 24 deletions(-) diff --git a/server/conf.c b/server/conf.c index e6046ed7c9..8d925c9fcb 100644 --- a/server/conf.c +++ b/server/conf.c @@ -116,44 +116,40 @@ static void ups_update(const char *fn, const char *name, const char *desc) /* return 1 if usable, 0 if not */ static int parse_upsd_conf_args(int numargs, char **arg) { + unsigned int temp; + /* everything below here uses up through arg[1] */ if (numargs < 2) return 0; /* MAXAGE */ if (!strcmp(arg[0], "MAXAGE")) { - if (isdigit(arg[1])) { - maxage = atoi(arg[1]); + if (str_to_uint(arg[1], &temp, 10)) { + maxage = temp; return 1; } - else { - upslogx(LOG_ERR, "MAXAGE has non numeric value (%s)!", arg[1]); - return 0; - } + upslogx(LOG_ERR, "Could not convert MAXAGE (%s) to an unsigned int (%s)!", arg[1], strerror(errno)); + return 0; } /* TRACKINGDELAY */ if (!strcmp(arg[0], "TRACKINGDELAY")) { - if (isdigit(arg[1])) { - tracking_delay = atoi(arg[1]); + if (str_to_uint(arg[1], &temp, 10)) { + tracking_delay = temp; return 1; } - else { - upslogx(LOG_ERR, "TRACKINGDELAY has non numeric value (%s)!", arg[1]); - return 0; - } + upslogx(LOG_ERR, "Could not convert TRACKINGDELAY (%s) to an unsigned int (%s)!", arg[1], strerror(errno)); + return 0; } /* MAXCONN */ if (!strcmp(arg[0], "MAXCONN")) { - if (isdigit(arg[1])) { - maxconn = atoi(arg[1]); + if (str_to_uint(arg[1], &temp, 10)) { + maxconn = temp; return 1; } - else { - upslogx(LOG_ERR, "MAXCONN has non numeric value (%s)!", arg[1]); - return 0; - } + upslogx(LOG_ERR, "Could not convert MAXCONN (%s) to an unsigned int (%s)!", arg[1], strerror(errno)); + return 0; } /* STATEPATH */ @@ -187,14 +183,12 @@ static int parse_upsd_conf_args(int numargs, char **arg) #ifdef WITH_CLIENT_CERTIFICATE_VALIDATION /* CERTREQUEST (0 | 1 | 2) */ if (!strcmp(arg[0], "CERTREQUEST")) { - if (isdigit(arg[1])) { - certrequest = atoi(arg[1]); + if (str_to_uint(arg[1], &temp, 10)) { + certrequest = temp; return 1; } - else { - upslogx(LOG_ERR, "CERTREQUEST has non numeric value (%s)!", arg[1]); - return 0; - } + upslogx(LOG_ERR, "Could not convert CERTREQUEST (%s) to an unsigned int (%s)!", arg[1], strerror(errno)); + return 0; } #endif /* WITH_CLIENT_CERTIFICATE_VALIDATION */ #endif /* WITH_OPENSSL | WITH_NSS */ From f88b0876527ce43f9a10642181ca5470bbee9ace Mon Sep 17 00:00:00 2001 From: Arnaud Quette Date: Fri, 15 Mar 2019 09:16:39 +0100 Subject: [PATCH 02/10] Change variable type from signed to unsigned int Signed-off-by: Arnaud Quette --- server/netssl.c | 2 +- server/netssl.h | 2 +- server/sstate.c | 2 +- server/sstate.h | 2 +- server/upsd.c | 13 +++++++------ server/upsd.h | 2 +- 6 files changed, 12 insertions(+), 11 deletions(-) diff --git a/server/netssl.c b/server/netssl.c index c2f409899d..d1c818e261 100644 --- a/server/netssl.c +++ b/server/netssl.c @@ -46,7 +46,7 @@ char *certname = NULL; char *certpasswd = NULL; #ifdef WITH_CLIENT_CERTIFICATE_VALIDATION -int certrequest = 0; +unsigned int certrequest = 0; #endif /* WITH_CLIENT_CERTIFICATE_VALIDATION */ static int ssl_initialized = 0; diff --git a/server/netssl.h b/server/netssl.h index 8f68168969..33f73ade0f 100644 --- a/server/netssl.h +++ b/server/netssl.h @@ -32,7 +32,7 @@ extern char *certfile; extern char *certname; extern char *certpasswd; #ifdef WITH_CLIENT_CERTIFICATE_VALIDATION -extern int certrequest; +extern unsigned int certrequest; #endif /* WITH_CLIENT_CERTIFICATE_VALIDATION */ /* List possible values for certrequested */ diff --git a/server/sstate.c b/server/sstate.c index 0365cd30f6..28910dbf3e 100644 --- a/server/sstate.c +++ b/server/sstate.c @@ -341,7 +341,7 @@ const cmdlist_t *sstate_getcmdlist(const upstype_t *ups) return ups->cmdlist; } -int sstate_dead(upstype_t *ups, int maxage) +int sstate_dead(upstype_t *ups, unsigned int maxage) { time_t now; double elapsed; diff --git a/server/sstate.h b/server/sstate.h index fe8485ced3..0b274c108c 100644 --- a/server/sstate.h +++ b/server/sstate.h @@ -47,7 +47,7 @@ const cmdlist_t *sstate_getcmdlist(const upstype_t *ups); void sstate_makeinfolist(const upstype_t *ups, char *buf, size_t bufsize); void sstate_makerwlist(const upstype_t *ups, char *buf, size_t bufsize); void sstate_makeinstcmdlist_t(const upstype_t *ups, char *buf, size_t bufsize); -int sstate_dead(upstype_t *ups, int maxage); +int sstate_dead(upstype_t *ups, unsigned int maxage); void sstate_infofree(upstype_t *ups); void sstate_cmdfree(upstype_t *ups); int sstate_sendline(upstype_t *ups, const char *buf); diff --git a/server/upsd.c b/server/upsd.c index a49b51110f..855a3ce0fe 100644 --- a/server/upsd.c +++ b/server/upsd.c @@ -52,13 +52,13 @@ int deny_severity = LOG_WARNING; upstype_t *firstups = NULL; /* default 15 seconds before data is marked stale */ - int maxage = 15; + unsigned int maxage = 15; /* default to 1h before cleaning up status tracking entries */ - int tracking_delay = 3600; + unsigned int tracking_delay = 3600; /* preloaded to {OPEN_MAX} in main, can be overridden via upsd.conf */ - int maxconn = 0; + unsigned int maxconn = 0; /* preloaded to STATEPATH in main, can be overridden via upsd.conf */ char *statepath = NULL; @@ -691,7 +691,7 @@ static void upsd_cleanup(void) void poll_reload(void) { - int ret; + unsigned int ret; ret = sysconf(_SC_OPEN_MAX); @@ -922,7 +922,8 @@ int nut_uuid_v4(char *uuid_str) /* service requests and check on new data */ static void mainloop(void) { - int i, ret, nfds = 0; + unsigned int i, nfds = 0; + int ret; upstype_t *ups; nut_ctype_t *client, *cnext; @@ -1246,7 +1247,7 @@ int main(int argc, char **argv) chroot_start(chroot_path); } - /* default to system limit (may be overridden in upsd.conf */ + /* default to system limit (may be overridden in upsd.conf) */ maxconn = sysconf(_SC_OPEN_MAX); /* handle upsd.conf */ diff --git a/server/upsd.h b/server/upsd.h index 2cd3eb399e..7bff8a5f75 100644 --- a/server/upsd.h +++ b/server/upsd.h @@ -91,7 +91,7 @@ int tracking_is_enabled(void); /* declarations from upsd.c */ -extern int maxage, maxconn, tracking_delay; +extern unsigned int maxage, maxconn, tracking_delay; extern char *statepath, *datapath; extern upstype_t *firstups; extern nut_ctype_t *firstclient; From 3d79006c2ec3b038f71e1d9441aaf73f744b77ab Mon Sep 17 00:00:00 2001 From: Arnaud Quette Date: Fri, 15 Mar 2019 09:20:12 +0100 Subject: [PATCH 03/10] Simplify error logging Signed-off-by: Arnaud Quette --- server/conf.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/server/conf.c b/server/conf.c index 8d925c9fcb..6243c936ef 100644 --- a/server/conf.c +++ b/server/conf.c @@ -128,7 +128,7 @@ static int parse_upsd_conf_args(int numargs, char **arg) maxage = temp; return 1; } - upslogx(LOG_ERR, "Could not convert MAXAGE (%s) to an unsigned int (%s)!", arg[1], strerror(errno)); + upslogx(LOG_ERR, "Could not convert %s (%s) to an unsigned int (%s)!", arg[0], arg[1], strerror(errno)); return 0; } @@ -138,7 +138,7 @@ static int parse_upsd_conf_args(int numargs, char **arg) tracking_delay = temp; return 1; } - upslogx(LOG_ERR, "Could not convert TRACKINGDELAY (%s) to an unsigned int (%s)!", arg[1], strerror(errno)); + upslogx(LOG_ERR, "Could not convert %s (%s) to an unsigned int (%s)!", arg[0], arg[1], strerror(errno)); return 0; } @@ -148,7 +148,7 @@ static int parse_upsd_conf_args(int numargs, char **arg) maxconn = temp; return 1; } - upslogx(LOG_ERR, "Could not convert MAXCONN (%s) to an unsigned int (%s)!", arg[1], strerror(errno)); + upslogx(LOG_ERR, "Could not convert %s (%s) to an unsigned int (%s)!", arg[0], arg[1], strerror(errno)); return 0; } @@ -187,7 +187,7 @@ static int parse_upsd_conf_args(int numargs, char **arg) certrequest = temp; return 1; } - upslogx(LOG_ERR, "Could not convert CERTREQUEST (%s) to an unsigned int (%s)!", arg[1], strerror(errno)); + upslogx(LOG_ERR, "Could not convert %s (%s) to an unsigned int (%s)!", arg[0], arg[1], strerror(errno)); return 0; } #endif /* WITH_CLIENT_CERTIFICATE_VALIDATION */ From cfc39657f23bc5760b9cfa88c07008436bf10f08 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Mon, 14 Sep 2026 18:05:29 +0200 Subject: [PATCH 04/10] tests/.gitignore: temporary dir from test_upsdconf [#3631] Signed-off-by: Jim Klimov --- tests/.gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/.gitignore b/tests/.gitignore index df4d6c0792..ced65105ed 100644 --- a/tests/.gitignore +++ b/tests/.gitignore @@ -19,6 +19,7 @@ /test_authconf.log /test_authconf.trs /test_upsdconf +/test_upsdconf-*/ /test_upsdconf.log /test_upsdconf.trs /selftest-rw/* From 42274809a3ee5f20dd29a122c291c32eb3e187c2 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Mon, 14 Sep 2026 18:06:56 +0200 Subject: [PATCH 05/10] tests/Makefile.am: remove temporary dirs from test_upsdconf [#3631] Signed-off-by: Jim Klimov --- tests/Makefile.am | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index 9a2a26b307..62b78b5027 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -410,8 +410,8 @@ MAINTAINERCLEANFILES = Makefile.in .dirstamp # NOTE: Do not clean ".deps" in SUBDIRS of the main project, # the root Makefile.am takes care of that! -#clean-local: -# $(AM_V_at)rm -rf $(builddir)/.deps +clean-local: + $(AM_V_at)rm -rf $(builddir)/test_upsdconf-*/ # Helper for only the enabled libs to get built: all-libs-local: $(lib_LTLIBRARIES) $(noinst_LTLIBRARIES) $(EXTRA_LTLIBRARIES) From 265639870630ed9eeca46f0c2e3aaaee9b2698ab Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Mon, 14 Sep 2026 18:07:27 +0200 Subject: [PATCH 06/10] tests/test_upsdconf.c: log (and count) failures more visibly [#3631] Signed-off-by: Jim Klimov --- tests/test_upsdconf.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/test_upsdconf.c b/tests/test_upsdconf.c index 136dcc9527..22b09ecc68 100644 --- a/tests/test_upsdconf.c +++ b/tests/test_upsdconf.c @@ -30,8 +30,8 @@ int certrequest = 0; /* These daemon operations must not be reached by the configuration tests. */ static void unexpected_call(const char *name) { - upslogx(LOG_ERR, "Unexpected %s", name); failures++; + upslogx(LOG_ERR, "FAILURE [#%d]\tUnexpected %s", failures, name); } upstype_t *get_ups_ptr(const char *name) @@ -112,9 +112,10 @@ static void check_value(const char *option, const char *value, int accepted, uin actual = stored_value(option); checks++; if (result != accepted || actual != expected) { - upslogx(LOG_ERR, "FAIL %s '%s': accepted=%d, value=%" PRIuMAX - "; expected %d, %" PRIuMAX, option, value, result, actual, accepted, expected); failures++; + upslogx(LOG_ERR, "FAILURE [#%d]\t%s '%s': accepted=%d, value=%" PRIuMAX + "; expected %d, %" PRIuMAX, + failures, option, value, result, actual, accepted, expected); } free(args[0]); free(args[1]); @@ -138,9 +139,10 @@ static void check_value(const char *option, const char *value, int accepted, uin actual = stored_value(option); checks++; if (actual != expected) { - upslogx(LOG_ERR, "FAIL load_upsdconf(%d), %s '%s': value=%" PRIuMAX - "; expected %" PRIuMAX, reloading, option, value, actual, expected); failures++; + upslogx(LOG_ERR, "FAILURE [#%d]\tload_upsdconf(%d), %s '%s': value=%" PRIuMAX + "; expected %" PRIuMAX, + failures, reloading, option, value, actual, expected); } } } @@ -232,8 +234,8 @@ int main(void) ups.last_ping = ups.last_heard; checks++; if (sstate_dead(&ups, INT_MAX)) { - upslogx(LOG_ERR, "FAIL MAXAGE INT_MAX: fresh driver data is stale"); failures++; + upslogx(LOG_ERR, "FAILURE [#%d]\tMAXAGE INT_MAX: fresh driver data is stale", failures); } free(ups.name); #endif From b79b8cb7e54f21401f78bfb01db7c8017c361919 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Mon, 14 Sep 2026 18:12:06 +0200 Subject: [PATCH 07/10] tests/test_upsdconf.c: adapt to MAXAGE and TRACKINGDELAY as UINT now [#3631, #677] Signed-off-by: Jim Klimov --- tests/test_upsdconf.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/test_upsdconf.c b/tests/test_upsdconf.c index 22b09ecc68..bea66cba06 100644 --- a/tests/test_upsdconf.c +++ b/tests/test_upsdconf.c @@ -195,6 +195,9 @@ int main(void) check_value(options[i], invalid[j], 0, previous); limit = (uintmax_t)INT_MAX; + if (!strcmp(options[i], "MAXAGE") || !strcmp(options[i], "TRACKINGDELAY")) { + limit = (uintmax_t)UINT_MAX; + } else if (!strcmp(options[i], "MAXCONN")) { limit = (uintmax_t)((nfds_t)-1); if (limit > (uintmax_t)LONG_MAX) limit = (uintmax_t)LONG_MAX; From 8056925f6c1816aea02db9fb2bd68cd327e48c4a Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Mon, 26 Oct 2026 14:10:48 +0100 Subject: [PATCH 08/10] server/conf.c: parse_upsd_conf_args(): refactor with invalid_number_out_of_range and invalid_notnumber_notboolean standardized reports [#677] Signed-off-by: Jim Klimov --- server/conf.c | 41 ++++++++++++++++++----------------------- 1 file changed, 18 insertions(+), 23 deletions(-) diff --git a/server/conf.c b/server/conf.c index ca70ac7986..e7a55ec96c 100644 --- a/server/conf.c +++ b/server/conf.c @@ -186,7 +186,7 @@ static int parse_upsd_conf_args(size_t numargs, char **arg) if ( str_to_int (arg[1], &lvl, 10) && lvl >= 0 ) { nut_debug_level_global = lvl; } else { - upslogx(LOG_INFO, "DEBUG_MIN has non numeric or negative value in upsd.conf"); + upslogx(LOG_INFO, "DEBUG_MIN has non-numeric or negative value in upsd.conf"); } return 1; } @@ -205,11 +205,7 @@ static int parse_upsd_conf_args(size_t numargs, char **arg) maxage = u; return 1; } - else { - upslogx(LOG_ERR, "%s has invalid or out of range numeric value (%s)! %s", - arg[0], arg[1], errno ? strerror(errno) : ""); - return 0; - } + goto invalid_number_out_of_range; } /* TRACKINGDELAY */ @@ -220,11 +216,7 @@ static int parse_upsd_conf_args(size_t numargs, char **arg) tracking_delay = u; return 1; } - else { - upslogx(LOG_ERR, "%s has invalid or out of range numeric value (%s)! %s", - arg[0], arg[1], errno ? strerror(errno) : ""); - return 0; - } + goto invalid_number_out_of_range; } /* ALLOW_NO_DEVICE */ @@ -236,8 +228,7 @@ static int parse_upsd_conf_args(size_t numargs, char **arg) if (parse_boolean(arg[1], &allow_no_device)) return 1; - upslogx(LOG_ERR, "%s has non numeric and non boolean value (%s)!", arg[0], arg[1]); - return 0; + goto invalid_notnumber_notboolean; } /* ALLOW_NOT_ALL_LISTENERS */ @@ -249,8 +240,7 @@ static int parse_upsd_conf_args(size_t numargs, char **arg) if (parse_boolean(arg[1], &allow_not_all_listeners)) return 1; - upslogx(LOG_ERR, "%s has non numeric and non boolean value (%s)!", arg[0], arg[1]); - return 0; + goto invalid_notnumber_notboolean; } /* MAXCONN */ @@ -261,11 +251,7 @@ static int parse_upsd_conf_args(size_t numargs, char **arg) maxconn = (nfds_t)l; return 1; } - else { - upslogx(LOG_ERR, "%s has invalid or out of range numeric value (%s)! %s", - arg[0], arg[1], errno ? strerror(errno) : ""); - return 0; - } + goto invalid_number_out_of_range; } /* STATEPATH */ @@ -351,9 +337,7 @@ static int parse_upsd_conf_args(size_t numargs, char **arg) certrequest = NETSSL_CERTREQ_REQUIRE; /* 2 */ return 1; } - upslogx(LOG_ERR, "%s has invalid or out of range numeric value (%s)! %s", - arg[0], arg[1], errno ? strerror(errno) : ""); - return 0; + goto invalid_number_out_of_range; } } # else /* !WITH_CLIENT_CERTIFICATE_VALIDATION */ @@ -432,6 +416,17 @@ static int parse_upsd_conf_args(size_t numargs, char **arg) /* not recognized */ return 0; + +invalid_number_out_of_range: + upslogx(LOG_ERR, "%s has invalid or out of range numeric value (%s)! %s", + arg[0], arg[1], errno ? strerror(errno) : ""); + return 0; + +invalid_notnumber_notboolean: + upslogx(LOG_ERR, "%s has non numeric and non boolean value (%s)!", + arg[0], arg[1]); + return 0; + } /* called for fatal errors in parseconf like malloc failures */ From c6f5ad65a391537b490498795b3296a58737a62e Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Mon, 26 Oct 2026 14:11:24 +0100 Subject: [PATCH 09/10] server/conf.c: parse_boolean(): assert that `arg` and `result` pointers are valid [#677] Signed-off-by: Jim Klimov --- server/conf.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/server/conf.c b/server/conf.c index e7a55ec96c..555421c549 100644 --- a/server/conf.c +++ b/server/conf.c @@ -151,6 +151,8 @@ static void ups_update(const char *fn, const char *name, const char *desc) */ static int parse_boolean(char *arg, int *result) { + assert(arg); + assert(result); if ( (!strcasecmp(arg, "true")) || (!strcasecmp(arg, "on")) || (!strcasecmp(arg, "yes")) || (!strcasecmp(arg, "1"))) { *result = 1; return 1; From 71495ae43d1de7a7878af1bd10947cccc51d1c6f Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Mon, 26 Oct 2026 16:38:50 +0100 Subject: [PATCH 10/10] tests/test_upsdconf.c: fix platform-dependent (32/64-bit) test expectations [#677, #3840] Signed-off-by: Jim Klimov --- tests/test_upsdconf.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/test_upsdconf.c b/tests/test_upsdconf.c index c44d99a8dc..8e996954b5 100644 --- a/tests/test_upsdconf.c +++ b/tests/test_upsdconf.c @@ -293,7 +293,9 @@ int main(void) } else if (!strcmp(options[i], "MAXCONN")) { limit = (uintmax_t)((nfds_t)-1); - if (limit > (uintmax_t)LONG_MAX) limit = (uintmax_t)LONG_MAX; + if (limit > (uintmax_t)LONG_MAX) { + limit = (uintmax_t)LONG_MAX; + } } else if (!strcmp(options[i], "CERTREQUEST")) { limit = (uintmax_t)(NETSSL_CERTREQ_MAX); @@ -304,8 +306,14 @@ int main(void) check_value(options[i], number, 1, limit); snprintf(number, sizeof(number), "%" PRIuMAX, limit + 1); check_value(options[i], number, 0, previous); + + /* Platform-dependent (32/64-bit ints) - value may overflow the fixed max or not */ snprintf(number, sizeof(number), "%" PRIuMAX, (uintmax_t)LONG_MAX + 1); - check_value(options[i], number, 0, previous); + if ((uintmax_t)LONG_MAX < limit) { + check_value(options[i], number, 1, (uintmax_t)LONG_MAX + 1); + } else { + check_value(options[i], number, 0, previous); + } } #if defined(WITH_SSL) && defined(WITH_CLIENT_CERTIFICATE_VALIDATION) check_value("CERTREQUEST", "NO", 1, 0);