Merge branch 'main' of github.com:performancecopilot/pcp

natoscott · natoscott · commit f7aaf6ac7f0f · 2025-07-03T12:01:10.000+10:00
diff --git a/qa/1605 b/qa/1605
@@ -0,0 +1,38 @@
+#!/bin/sh
+# PCP QA Test No. 1605
+# pmGetOpt - mem leak from ambiguity handling of long options
+#
+# Copyright (c) 2025 Ken McDonell.  All Rights Reserved.
+#
+
+if [ $# -eq 0 ]
+then
+    seq=`basename $0`
+    echo "QA output created by $seq"
+else
+    # use $seq from caller, unless not set
+    [ -n "$seq" ] || seq=`basename $0`
+    echo "QA output created by `basename $0` $*"
+fi
+
+# get standard environment, filters and checks
+. ./common.product
+. ./common.filter
+. ./common.check
+
+_check_valgrind
+
+_cleanup()
+{
+    cd $here
+    $sudo rm -rf $tmp $tmp.*
+}
+
+status=0	# success is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# real QA test starts here
+_run_valgrind pminfo --h
+
+# success, all done
+exit
diff --git a/qa/1605.out b/qa/1605.out
@@ -0,0 +1,53 @@
+QA output created by 1605
+=== std out ===
+=== std err ===
+pminfo: option '--h' is ambiguous; possibilities: '--host' '--helptext' '--help' '--hostzone'
+Usage: pminfo [options] [metricname | pmid | indom]...
+
+General options:
+  -a FILE, --archive=FILE
+                        metrics source is a PCP archive
+  -D DBG, --debug=DBG   set debug options, see pmdbg(1)
+  -h HOST, --host=HOST  metrics source is PMCD on host
+  --container=NAME      specify an individual container to be queried
+  -L, --local-PMDA      metrics source is local connection to a PMDA
+  -K SPEC, --spec-local=SPEC
+                        optional additional PMDA spec for local connection
+  -n FILE, --namespace=FILE
+                        use an alternative PMNS
+  -N FILE, --uniqnames=FILE
+                        like -n but only one name allowed for each PMID
+  -O TIME, --origin=TIME
+                        initial sample time within the time window
+  -Z TZ, --timezone=TZ  set reporting timezone
+  -z, --hostzone        set reporting timezone to local time of metrics source
+  -V, --version         display version number and exit
+  -?, --help            show this usage message and exit
+
+Protocol options:
+  -b N, --batch=N       fetch N metrics at a time for -f and -v [128]
+  -d, --desc            get and print metric description
+  -f, --fetch           fetch and print values for all instances
+  -F, --fetchall        fetch and print values for non-enumerable indoms
+  -I, --fullindom       print InDom in verbose format
+  -l, --labels          print InDom, metric and instance labels
+  -m, --pmid            print PMID
+  -M, --fullpmid        print PMID in verbose format
+  -s, --series          print source, metric, instance series identifiers
+  -t, --oneline         get and display (terse) oneline text
+  -T, --helptext        get and display (verbose) help text
+
+Metrics options:
+  -c FILE, --derived=FILE
+                        load global derived metric definitions from FILE(s)
+  -r name=expr, --register=name=expr
+                        register a per-context derived metric
+  -x, --events          unpack and report on any fetched event records
+  -v, --verify          verify mode, be quiet and only report errors
+=== filtered valgrind report ===
+Memcheck, a memory error detector
+Command: pminfo --h
+LEAK SUMMARY:
+definitely lost: 0 bytes in 0 blocks
+indirectly lost: 0 bytes in 0 blocks
+ERROR SUMMARY: 0 errors from 0 contexts ...
diff --git a/qa/1615 b/qa/1615
@@ -35,6 +35,12 @@ _cleanup()
     _restore_config $PCP_SYSCONF_DIR/pmproxy/pmproxy.conf
 
     [ -d $archive_path ] && $sudo rm -fr $archive_path
+    if [ -f $PCP_LOG_DIR/pmproxy/pmproxy.log ]
+    then
+	cat $PCP_LOG_DIR/pmproxy/pmproxy.log >>$seq_full
+    else
+	echo "Arrg, $PCP_LOG_DIR/pmproxy/pmproxy.log missing"
+    fi
     if $pmproxy_was_running
     then
         echo "Restart pmproxy ..." >>$seq_full
diff --git a/qa/1617 b/qa/1617
@@ -166,10 +166,10 @@ pmseries -c $tmp.conf -m | _filter_metrics
 # valgrind takes awhile to shutdown too
 _wait_process_end $pid
 echo "=== valgrind stdout ===" | tee -a $seq_full
-cat $tmp.valout | _filter_valgrind
+cat $tmp.valout | tee -a $seq_full | _filter_valgrind
 
 echo "=== valgrind stderr ===" | tee -a $seq_full
-cat $tmp.valerr | _filter_pmproxy_log | grep -v "Cannot connect to key server" | _filter_pmproxy_port
+cat $tmp.valerr | tee -a $seq_full | _filter_pmproxy_log | grep -v "Cannot connect to key server" | _filter_pmproxy_port
 
 # success, all done
 exit
diff --git a/qa/761 b/qa/761
@@ -39,6 +39,7 @@ root_cleanup()
     [ -f $tmp.conf.backup ] && $sudo cp $tmp.conf.backup $PCP_DIR/etc/pcp.conf
     _restore_pmda_install root || _exit 1
     rm -f $here/$seq.test-lxc-info.sh
+    rm -rf $tmp.* $tmp
 }
 
 # do metrics one at a time, map all internal instance ids to N and sort
diff --git a/qa/801 b/qa/801
@@ -19,7 +19,7 @@ perl -e "use PCP::PMDA" >/dev/null 2>&1
 [ $? -eq 0 ] || _notrun "perl PCP::PMDA module not installed"
 
 status=1	# failure is the default!
-trap "cd $here; rm -rf $tmp.*; exit \$status" 0 1 2 3 15
+trap "cd $here; rm -rf $tmp $tmp.*; exit \$status" 0 1 2 3 15
 
 #setup the tmp proc file system for older config
 pmdanetfilter_prepare()
diff --git a/qa/864 b/qa/864
@@ -23,6 +23,7 @@ if grep 'collectl version' $tmp.out >/dev/null
 then
     :
 else
+    rm -f $tmp.out
     _notrun "pcp-collectl not installed"
 fi
 
diff --git a/qa/README.vm b/qa/README.vm
@@ -585,3 +585,12 @@ Recreate the mirror disk on vm03
 UUID=8f865731-443c-4b84-ade7-31e6ff94ee7c /silver ext2 defaults 0 0
     $ sudo systemctl daemon-reload
     $ sudo mount -a
+
+Adding a new disk
+
+    $ disk=/dev/vdb
+    $ sudo cfdisk $disk         <- make partition 1, all of the disk
+    $ mkfs.xfs ${disk}1
+    $ lsblk -fs ${disk}1 	-> UUID=<uuid>
+    $ [ -d /data ] || sudo mkdir /data
+    $ echo "UUID=<uuid> /data xfs default 0 2" | sudo tee -a /etc/fstab
diff --git a/qa/group b/qa/group
@@ -2093,6 +2093,7 @@ pmcd.pdu
 1602 pmproxy local
 1603 pmproxy local
 1604 pmseries pmproxy local
+1605 libpcp pminfo valgrind local
 1608 pmproxy local
 1610 libpcp local
 1611 pmlogger pmproxy pmlogpush libpcp_web local
@@ -2242,7 +2243,7 @@ pmcd.pdu
 1963 pmda.linux local
 1970 pmda.bpf local
 1973 pcp zoneinfo python local
-1976 pmdaopenmetrics python local
+1976 pmda.openmetrics python local
 1978 atop local pmlogrewrite
 1984 pmlogconf pmda.redis local
 1985 pmfind local valgrind
diff --git a/src/libpcp/src/e_loglabel.c b/src/libpcp/src/e_loglabel.c
@@ -31,8 +31,7 @@
 typedef struct {
     __uint32_t	magic;		/* PM_LOG_MAGIC|PM_LOG_VERS02 */
     __int32_t	pid;		/* PID of logger */
-    __int32_t	start_sec;	/* start of this log (pmTimeval) */
-    __int32_t	start_usec;
+    __int32_t	start[2];	/* start time of this archive (pmTimeval) */
     __int32_t	vol;		/* current log volume no. */
     char	hostname[PM_LOG_MAXHOSTLEN]; /* name of collection host */
     char	timezone[PM_TZ_MAXLEN];	/* $TZ at collection host */
@@ -44,8 +43,7 @@ typedef struct {
 typedef struct {
     __uint32_t	magic;		/* PM_LOG_MAGIC|PM_LOG_VERS03 */
     __int32_t	pid;		/* PID of logger */
-    __int32_t	start_sec[2];	/* start of this log (__pmTimestamp) */
-    __int32_t	start_nsec;
+    __int32_t	start[3];	/* start time of this archive (__pmTimestamp) */
     __int32_t	vol;		/* current log volume no. */
     __uint32_t	features;	/* enabled archive feature bits */
     __uint32_t	reserved;	/* reserved for future use, zero padded */
@@ -115,7 +113,7 @@ __pmLogEncodeLabel(const __pmLogLabel *lp, void **buffer, size_t *length)
 	/* swab */
 	label.magic = htonl(lp->magic);
 	label.pid = htonl(lp->pid);
-	__pmPutTimestamp(&lp->start, &label.start_sec[0]);
+	__pmPutTimestamp(&lp->start, label.start);
 	label.vol = htonl(lp->vol);
 	label.features = htonl(lp->features);
 	label.reserved = 0;
@@ -139,7 +137,7 @@ __pmLogEncodeLabel(const __pmLogLabel *lp, void **buffer, size_t *length)
 	/* swab */
 	label.magic = htonl(lp->magic);
 	label.pid = htonl(lp->pid);
-	__pmPutTimeval(&lp->start, &label.start_sec);
+	__pmPutTimeval(&lp->start, label.start);
 	label.vol = htonl(lp->vol);
 	memset(label.hostname, 0, sizeof(label.hostname));
 	bytes = MINIMUM(strlen(lp->hostname), PM_LOG_MAXHOSTLEN - 1);
@@ -187,7 +185,7 @@ __pmLogDecodeLabelV3(__pmLabel_v3 *in, __pmLogLabel *lp)
 {
     __pmLogFreeLabel(lp);	/* reset from earlier call */
     lp->pid = ntohl(in->pid);
-    __pmLoadTimestamp(&in->start_sec[0], &lp->start);
+    __pmLoadTimestamp(in->start, &lp->start);
     lp->vol = ntohl(in->vol);
     lp->features = ntohl(in->features);
     lp->hostname = strndup(in->hostname, PM_MAX_HOSTNAMELEN - 1);
@@ -200,7 +198,7 @@ __pmLogDecodeLabelV2(__pmLabel_v2 *in, __pmLogLabel *lp)
 {
     __pmLogFreeLabel(lp);	/* reset from earlier call */
     lp->pid = ntohl(in->pid);
-    __pmLoadTimeval(&in->start_sec, &lp->start);
+    __pmLoadTimeval(in->start, &lp->start);
     lp->vol = ntohl(in->vol);
     lp->features = 0;		/* not supported in v2 */
     lp->hostname = strndup(in->hostname, PM_LOG_MAXHOSTLEN - 1);
diff --git a/src/libpcp/src/getopt.c b/src/libpcp/src/getopt.c
@@ -1362,6 +1362,20 @@ typedef struct pmOptList {
     struct pmOptList *	next;
 } pmOptionsList;
 
+static void
+free_list(pmOptionsList *head)
+{
+    pmOptionsList	*prior = NULL;
+    while (head != NULL) {
+	if (prior != NULL)
+	    free(prior);
+	prior = head;
+	head = head->next;
+    }
+    if (prior != NULL)
+	free(prior);
+}
+
 int
 pmgetopt_r(int argc, char *const *argv, pmOptions *d)
 {
@@ -1538,26 +1552,27 @@ pmgetopt_r(int argc, char *const *argv, pmOptions *d)
 	if (ambig_list != NULL && !exact) {
 	    if (print_errors) {
 		pmOptionsList first;
+		pmOptionsList *olp;
 		first.p = pfound;
 		first.next = ambig_list;
-		ambig_list = &first;
+		olp = &first;
 
 		pmprintf("%s: option '%s' is ambiguous; possibilities:",
 			pmGetProgname(), argv[d->optind]);
 		do {
-		    pmprintf(" '--%s'", ambig_list->p->long_opt);
-		    ambig_list = ambig_list->next;
-		} while (ambig_list != NULL);
+		    pmprintf(" '--%s'", olp->p->long_opt);
+		    olp = olp->next;
+		} while (olp != NULL);
 		pmprintf("\n");
 	    }
 	    d->__nextchar += strlen(d->__nextchar);
 	    d->optind++;
 	    d->optopt = 0;
-	    free(ambig_list);
+	    free_list(ambig_list);
 	    return '?';
 	}
 	else if (ambig_list != NULL) {
-	    free(ambig_list);
+	    free_list(ambig_list);
 	}
 
 	if (pfound != NULL) {
diff --git a/src/libpcp/src/ipc.c b/src/libpcp/src/ipc.c
@@ -179,6 +179,12 @@ __pmSetVersionIPC(int fd, int version)
     if (pmDebugOptions.context)
 	fprintf(stderr, "__pmSetVersionIPC: fd=%d version=%d\n", fd, version);
 
+    if (fd < 0) {
+	/* this should never happen */
+	pmNotifyErr(LOG_WARNING, "%s: fd %d not valid", __FUNCTION__, fd);
+	return -EBADF;
+    }
+
     PM_LOCK(ipc_lock);
     if ((sts = resize(fd)) < 0) {
 	PM_UNLOCK(ipc_lock);
diff --git a/src/libpcp/src/rtime.c b/src/libpcp/src/rtime.c
@@ -934,9 +934,13 @@ pmParseTimeWindow(
 	    offset = end;
 	}
 
-	if (swAlign) {
-	    align = offset.tv_nsec + 1000000000 * (__int64_t)offset.tv_sec;
-	    blign = (align / delta) * delta;
+	/*
+	 * if end.tv_sec == PM_MAX_TIME_T then we cannot do -A rounding
+	 * due to arithmetic overflow [Coverity CID 464604]
+	 */
+	if (swAlign && end.tv_sec != PM_MAX_TIME_T) {
+	    align = offset.tv_nsec + 1000000000 *
+	    (__int64_t)offset.tv_sec; blign = (align / delta) * delta;
 	    if (blign < align)
 		blign += delta;
 	    align = end.tv_nsec + 1000000000 * (__int64_t)end.tv_sec;
diff --git a/src/libpcp_web/src/discover.c b/src/libpcp_web/src/discover.c
@@ -1526,8 +1526,6 @@ pmDiscoverStreamData(pmDiscover *p, const char *content, size_t length)
 	return PM_ERR_NOCONTEXT;
     }
     pmUseContext(p->ctx);
-    ctxp = __pmHandleToPtr(p->ctx);
-    PM_UNLOCK(ctxp->c_lock);
 
     p->datavol = sdscatlen(p->datavol, content, length);
     bytes = sdslen(p->datavol);
@@ -1562,8 +1560,10 @@ pmDiscoverStreamData(pmDiscover *p, const char *content, size_t length)
 	pb[0] = buflen + sizeof(uint32_t);
 	pb[1] = pb[2] = 0;	/* type, from */
 	memcpy(&pb[3], buf + sizeof(uint32_t), buflen - (sizeof(uint32_t) * 2));
+	ctxp = __pmHandleToPtr(p->ctx);
 	sts = __pmDecodeResult_ctx(ctxp, pb, &rp);
 	__pmUnpinPDUBuf(pb);
+	PM_UNLOCK(ctxp->c_lock);
 	if (sts < 0) {
 	    infofmt(msg, "Failed decoding %u bytes result buffer: %s\n",
 			    buflen, pmErrStr(sts));
diff --git a/src/pmdas/cisco/probe.c b/src/pmdas/cisco/probe.c
@@ -107,16 +107,29 @@ probe_cisco(cisco_t * cp)
     if (cp->fin == NULL) {
 	fd = conn_cisco(cp);
 	if (fd == -1) {
-	    fprintf(stderr, "grab_cisco(%s): connect failed: %s\n",
+	    fprintf(stderr, "probe_cisco(%s): connect failed: %s\n",
 		cp->host, osstrerror());
 	    return;
 	}
 	else {
 	    cp->fin = fdopen (fd, "r");
+	    if ((cp->fin = fdopen (fd, "r")) == NULL) {
+		fprintf(stderr, "probe_cisco(%s): input fdopen(%d) failed: %s\n",
+			    cp->host, fd, strerror(oserror()));
+		close(fd);
+		return;
+	    }
 	    if ((fd2 = dup(fd)) < 0) {
 	    	perror("dup");
 		exit(1);
 	    }
+	    if ((cp->fout = fdopen (fd2, "w")) == NULL) {
+		fprintf(stderr, "probe_cisco(%s): output fdopen(%d) failed: %s\n",
+			    cp->host, fd2, strerror(oserror()));
+		close(fd);
+		close(fd2);
+		return;
+	    }
 	    cp->fout = fdopen (fd2, "w");
 	    if (cp->username != NULL) {
 		/*
diff --git a/src/pmdas/cisco/telnet.c b/src/pmdas/cisco/telnet.c
@@ -436,12 +436,25 @@ grab_cisco(intf_t *ip)
 	    return -1;
 	}
 	else {
-	    cp->fin = fdopen (fd, "r");
+	    if ((cp->fin = fdopen (fd, "r")) == NULL) {
+		if (pmDebugOptions.appl0)
+		    fprintf(stderr, "grab_cisco(%s:%s): input fdopen(%d) failed: %s\n",
+			    cp->host, ip->interface, fd, strerror(oserror()));
+		close(fd);
+		return -1;
+	    }
 	    if ((fd2 = dup(fd)) < 0) {
 	    	perror("dup");
 		exit(1);
 	    }
-	    cp->fout = fdopen (fd2, "w");
+	    if ((cp->fout = fdopen (fd2, "w")) == NULL) {
+		if (pmDebugOptions.appl0)
+		    fprintf(stderr, "grab_cisco(%s:%s): output fdopen(%d) failed: %s\n",
+			    cp->host, ip->interface, fd2, strerror(oserror()));
+		close(fd);
+		close(fd2);
+		return -1;
+	    }
 	    if (pmDebugOptions.appl0) {
 		fprintf(stderr, "grab_cisco(%s:%s): connected fin=%d fout=%d",
 		    cp->host, ip->interface, fileno(cp->fin), fileno(cp->fout));
diff --git a/src/pmdas/hdb/pmdahdb.python b/src/pmdas/hdb/pmdahdb.python
diff --git a/src/pmdas/hdb/pmlogconf.summary b/src/pmdas/hdb/pmlogconf.summary
diff --git a/src/pmlogger/src/remote.c b/src/pmlogger/src/remote.c

Original file line number	Diff line number	Diff line change
`@@ -39,6 +39,7 @@ root_cleanup()`
`39`	`39`	`[ -f $tmp.conf.backup ] && $sudo cp $tmp.conf.backup $PCP_DIR/etc/pcp.conf`
`40`	`40`	`_restore_pmda_install root \|\| _exit 1`
`41`	`41`	`rm -f $here/$seq.test-lxc-info.sh`
	`42`	`+ rm -rf $tmp.* $tmp`
`42`	`43`	`}`
`43`	`44`
`44`	`45`	`# do metrics one at a time, map all internal instance ids to N and sort`
Original file line number	Diff line number	Diff line change
`@@ -23,6 +23,7 @@ if grep 'collectl version' $tmp.out >/dev/null`
`23`	`23`	`then`
`24`	`24`	`:`
`25`	`25`	`else`
	`26`	`+ rm -f $tmp.out`
`26`	`27`	`_notrun "pcp-collectl not installed"`
`27`	`28`	`fi`
`28`	`29`