Merge pull request #2528 from jimklimov/issue-2523

Implement parallel IPMI scanning in `nut-scanner`, and address some `valgrind` complaints

Makefile.am

@@ -92,6 +92,7 @@ distcheck-light-man:
 
 # Make a distcheck (and check in particular) with enabled valgrind and debug info
 memcheck distcheck-valgrind:
+	@echo "See also scripts/valgrind in NUT sources for a helper tool"
 	+$(MAKE) $(AM_MAKEFLAGS) DISTCHECK_FLAGS="$(DISTCHECK_VALGRIND_FLAGS)" distcheck
 
 # workaround the dist generated files that are also part of the distribution

NEWS.adoc

@@ -151,6 +151,8 @@ installed.
      subnets with too many bits available for the host address part (avoiding
      millions of scans in the extreme cases).
      [issue #2244, issue #2511, PR #2509, PR #2513, PR #2517]
+   * implemented parallel scanning for IPMI bus, otherwise default scan for
+     all supported buses with `-m auto` takes unbearably long. [#2523]
    * bumped version of `libnutscan` to 2.5.2, it now includes a few more
      methods and symbols from `libcommon`. [issue #2244, PR #2509]
 
@@ -182,6 +184,9 @@ installed.
    respective warnings issued by the new generations of analysis tools.
    [#823, #2437, link:https://github.com/networkupstools/nut-website/issues/52[nut-website issue #52]]
 
+ - added `scripts/valgrind` with a helper script and suppression file to
+   ignore common third-party problems. [#2511]
+
  - revised `nut.exe` (the NUT for Windows wrapper for all-in-one service)
    to be more helpful with command-line use (report that it failed to start
    as a service, have a help message, pass debug verbosity to launched NUT

UPGRADING.adoc

@@ -43,8 +43,8 @@ Changes from 2.8.2 to 2.8.3
 
 - Numerous changes to `nut-scanner` and symbols that its `libnutscan.so`
   delivers have caused a library version bump.  New methods have been added
-  in a (hopefully) backwards compatible manner. [issue #2244 and numerous
-  PRs for it]
+  and one structure (`nutscan_ipmi_t`) updated in a (hopefully) backwards
+  compatible manner. [PR #2523, issue #2244 and numerous PRs for it]
 
 - Internal API change for `sendsignalpid()` and `sendsignalfn()` methods,
   which can impact NUT forks which build using `libcommon.la` and similar

common/common.c

@@ -3087,6 +3087,9 @@ void nut_prepare_search_paths(void) {
 				dupe = 1;
 #if HAVE_DECL_REALPATH
 				free((char *)dirname);
+				/* Have some valid value, for kicks (likely
+				 * to be ignored in the code path below) */
+				dirname = search_paths_builtin[i];
 #endif
 				break;
 			}
@@ -3097,10 +3100,17 @@ void nut_prepare_search_paths(void) {
 				"existing unique directory: %s",
 				__func__, count_filtered, dirname);
 #if !HAVE_DECL_REALPATH
+			/* Make a copy of table entry, else we have
+			 * a dynamic result of realpath() made above.
+			 */
 			dirname = (const char *)xstrdup(dirname);
 #endif
 			filtered_search_paths[count_filtered++] = dirname;
-		}
+		}	/* else: dirname was freed above (for realpath)
+			 * or is a reference to the table entry; no need
+			 * to free() it either way */
+
+		closedir(dp);
 	}
 
 	/* If we mangled this before, forget the old result: */

docs/developers.txt

@@ -1212,6 +1212,9 @@ suspected area and then exit cleanly.  valgrind will tell you if you've
 done anything dodgy like freeing regions twice, reading uninitialized
 memory, or if you've leaked memory anywhere.
 
+See also `scripts/valgrind` in NUT sources for a helper tool and resource
+files to suppress common third-party problems.
+
 For more information, refer to the link:http://valgrind.kde.org[Valgrind]
 project.
 

docs/nut.dict

@@ -1,4 +1,4 @@
-personal_ws-1.1 en 3186 utf-8
+personal_ws-1.1 en 3188 utf-8
 AAC
 AAS
 ABI
@@ -2594,6 +2594,7 @@ pts
 pty
 pulizzi
 pw
+pwd
 pwmib
 px
 pxW
@@ -2859,6 +2860,7 @@ subnets
 subtree
 sudo
 suid
+suppressions
 suseconds
 sv
 svc

scripts/Makefile.am

@@ -25,12 +25,15 @@ EXTRA_DIST = \
     upower/95-upower-hid.rules \
     usb_resetter/README.adoc \
     usb_resetter/nut-driver.service \
+    valgrind/README.adoc \
+    valgrind/.valgrind.supp \
+    valgrind/valgrind.sh \
     Windows/halt.c \
     Windows/Makefile
 
 SUBDIRS = augeas devd hotplug installer python systemd udev ufw Solaris Windows upsdrvsvcctl
 
-SPELLCHECK_SRC = README.adoc RedHat/README.adoc usb_resetter/README.adoc
+SPELLCHECK_SRC = README.adoc RedHat/README.adoc usb_resetter/README.adoc valgrind/README.adoc
 
 # NOTE: Due to portability, we do not use a GNU percent-wildcard extension.
 # We also have to export some variables that may be tainted by relative

scripts/valgrind/.valgrind.supp

@@ -0,0 +1,142 @@
+{
+   <socketcall_sendto>
+   Memcheck:Param
+   socketcall.send(msg)
+   fun:send
+   ...
+}
+{
+   <glibc_freeres>
+   Memcheck:Free
+   fun:free
+   ...
+   fun:__libc_freeres
+   ...
+}
+{
+   <glibc_eh_alloc__see_bug_66339>
+   Memcheck:Leak
+   match-leak-kinds: reachable
+   fun:malloc
+   fun:pool
+   fun:__static_initialization_and_destruction_0
+   fun:_GLOBAL__sub_I_eh_alloc.cc
+   ...
+}
+{
+   <libgcrypt_init>
+   Memcheck:Leak
+   match-leak-kinds: reachable
+   fun:malloc
+   ...
+   fun:_gcry_mpi_init
+   fun:global_init
+   ...
+}
+{
+   <libssl_init_CRYPTO>
+   Memcheck:Leak
+   ...
+   fun:CRYPTO_*
+   ...
+}
+{
+   <libssl_init_SSL_CTX>
+   Memcheck:Leak
+   ...
+   fun:SSL_CTX_*
+   ...
+}
+{
+   <libssl_init_OPENSSL>
+   Memcheck:Leak
+   ...
+   fun:OPENSSL_*
+   ...
+}
+{
+   <ltdl_init>
+   Memcheck:Leak
+   fun:*alloc
+   ...
+   fun:_dl_init
+   ...
+}
+{
+   <ltdl_init>
+   Memcheck:Leak
+   fun:*alloc
+   ...
+   fun:_dl_new_object
+   ...
+}
+{
+   <ltdl_init>
+   Memcheck:Leak
+   fun:*alloc
+   ...
+   fun:elf_machine_rela
+   ...
+}
+{
+   <ltdl_init>
+   Memcheck:Leak
+   fun:*alloc
+   ...
+   fun:dl_open_worker
+   ...
+}
+{
+   <ltdl_init>
+   Memcheck:Leak
+   fun:*alloc
+   ...
+   fun:_dlerror_run
+   ...
+}
+{
+   <nut-scanner-snmp-libinit>
+   Memcheck:Leak
+   ...
+   fun:init_snmp_once
+   ...
+}
+{
+   <nut-scanner-snmp-sess_open>
+   Memcheck:Leak
+   fun:*alloc
+   ...
+   fun:wrap_nut_snmp_sess_open
+   ...
+}
+{
+   <nut-scanner-ipmi-ctxinit>
+   Memcheck:Leak
+   ...
+   fun:wrap_nut_ipmi_ctx_create
+   ...
+}
+{
+   <nut-scanner-avahi-clientinit>
+   Memcheck:Leak
+   ...
+   fun:wrap_nut_avahi_client_new
+   ...
+}
+{
+   # https://forums.freebsd.org/threads/named-semaphore-uninitialized-bytes.84850/
+   <glibc-sem_open>
+   Memcheck:Cond
+   ...
+   fun:sem_open
+   ...
+}
+{
+   <glibc-sem_trywait>
+   Memcheck:Cond
+   ...
+   fun:sem_trywait*
+   ...
+   fun:start_thread
+   ...
+}

scripts/valgrind/README.adoc

@@ -0,0 +1,16 @@
+VALGRIND resources
+==================
+
+Helper script and suppression file to analyze NUT binaries.
+
+Example use-case:
+----
+:; make -ks -j && LD_LIBRARY_PATH=`pwd`/clients/.libs \
+    ./scripts/valgrind/valgrind.sh ./tools/nut-scanner/nut-scanner -DDDDDD -m auto
+----
+
+See also:
+
+* link:https://wiki.wxwidgets.org/Valgrind_Suppression_File_Howto[Valgrind Suppression File How-to]
+  - Notably, add `--gen-suppressions=all --error-limit=no` to `valgrind`
+    program options to generate suppression snippets

scripts/valgrind/valgrind.sh

@@ -0,0 +1,21 @@
+#!/bin/sh
+
+# Copyright (C) 2024 by Jim Klimov <[email protected]>
+
+SCRIPTDIR="`dirname "$0"`"
+SCRIPTDIR="`cd "$SCRIPTDIR" && pwd`"
+
+LTEXEC=""
+[ -n "${LIBTOOL-}" ] || LIBTOOL="`command -v libtool`" 2>/dev/null
+[ -z "${LIBTOOL}"  ] || LTEXEC="${LIBTOOL} --mode=execute "
+
+# TODO: Also wrap tool=callgrind e.g. via $0 symlink name?
+
+exec ${LTEXEC} \
+valgrind \
+	--tool=memcheck --verbose \
+	--leak-check=full --show-reachable=yes --error-exitcode=1 --show-error-list=yes \
+	--trace-children=yes --track-fds=yes --show-leak-kinds=all --track-origins=yes \
+	--suppressions="${SCRIPTDIR}/.valgrind.supp" \
+	${VALGRIND_OPTIONS-} \
+	"$@"

tools/nut-scanner/Makefile.am

@@ -96,7 +96,7 @@ libnutscan_la_LDFLAGS += -version-info 2:5:2
 # copies of "nut_debug_level" making fun of our debug-logging attempts.
 # One solution to tackle if needed for those cases would be to make some
 # dynamic/shared libnutcommon (etc.)
-libnutscan_la_LDFLAGS += -export-symbols-regex '^(nutscan_|nut_debug_level|s_upsdebug|fatalx|xcalloc|snprintfcat|max_threads|curr_threads|nut_report_config_flags|upsdebugx_report_search_paths|nut_prepare_search_paths)'
+libnutscan_la_LDFLAGS += -export-symbols-regex '^(nutscan_|nut_debug_level|s_upsdebug|fatalx|fatal_with_errno|xcalloc|snprintfcat|max_threads|curr_threads|nut_report_config_flags|upsdebugx_report_search_paths|nut_prepare_search_paths)'
 libnutscan_la_CFLAGS = -I$(top_srcdir)/clients -I$(top_srcdir)/include \
 			$(LIBLTDL_CFLAGS) -I$(top_srcdir)/drivers
 

tools/nut-scanner/nut-scan.h

@@ -83,7 +83,7 @@ extern "C" {
 
 #ifdef HAVE_PTHREAD
 # if (defined HAVE_PTHREAD_TRYJOIN) || (defined HAVE_SEMAPHORE)
-extern size_t max_threads, curr_threads, max_threads_netxml, max_threads_oldnut, max_threads_netsnmp;
+extern size_t max_threads, curr_threads, max_threads_netxml, max_threads_oldnut, max_threads_netsnmp, max_threads_ipmi;
 # endif
 
 # ifdef HAVE_PTHREAD_TRYJOIN
@@ -110,16 +110,17 @@ typedef struct nutscan_snmp {
 } nutscan_snmp_t;
 
 /* IPMI structure */
-/* Settings for OutofBand (remote) connection */
+/* Settings for Out-of-Band (remote) connection */
 typedef struct nutscan_ipmi {
-	char*			username;            /* IPMI 1.5 and 2.0 */
-	char*			password;            /* IPMI 1.5 and 2.0 */
-	int				authentication_type; /* IPMI 1.5 */
-	int				cipher_suite_id;     /* IPMI 2.0 */
-	char*			K_g_BMC_key;         /* IPMI 2.0, optional key for 2 key auth. */
-	int				privilege_level;     /* for both */
+	char*		username;            /* IPMI 1.5 and 2.0 */
+	char*		password;            /* IPMI 1.5 and 2.0 */
+	int		authentication_type; /* IPMI 1.5 */
+	int		cipher_suite_id;     /* IPMI 2.0 */
+	char*		K_g_BMC_key;         /* IPMI 2.0, optional key for 2 key auth. */
+	int		privilege_level;     /* for both */
 	unsigned int	workaround_flags;    /* for both */
-	int				ipmi_version;        /* IPMI 1.5 or 2.0? */
+	int		ipmi_version;        /* IPMI 1.5 or 2.0? */
+	char*		peername;            /* Hostname or IP for remote scans, NULL for local device bus (populated by scanning methods) */
 } nutscan_ipmi_t;
 
 /* IPMI auth defines, simply using FreeIPMI defines */