Blob Blame History Raw
diff -Naurp pcp-4.1.0-orig/qa/1350 pcp-4.1.0/qa/1350
--- pcp-4.1.0-orig/qa/1350	1970-01-01 10:00:00.000000000 +1000
+++ pcp-4.1.0/qa/1350	2018-10-09 15:57:39.402661334 +1100
@@ -0,0 +1,38 @@
+#!/bin/sh
+# PCP QA Test No. 1350
+# Test fix for RHBZ #1600262 https://bugzilla.redhat.com/show_bug.cgi?id=1600262
+# pmdaproc short buffer read of /proc/PID/status yeilds incorrect metric values
+#
+# Copyright (c) 2018 Red Hat.  All Rights Reserved.
+#
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+# get standard environment, filters and checks
+. ./common.product
+. ./common.filter
+. ./common.check
+
+_cleanup()
+{
+    cd $here
+    $sudo rm -rf $tmp $tmp.*
+}
+
+[ $PCP_PLATFORM = linux ] || _notrun "Linux proc test, only works with Linux"
+
+status=1	# failure is the default!
+$sudo rm -rf $tmp $tmp.* $seq.full
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# real QA test starts here
+nvctxw=`pminfo -f proc.psinfo.nvctxsw | fgrep $$ | awk '{print $NF}'`
+[ $nvctxw -eq 0 ] || status=0
+if [ $status -eq 1 ]
+then
+    echo FAIL, proc.psinfo.nvctxsw should not be zero
+else
+    echo PASS, proc.psinfo.nvctxsw is nonzero
+fi
+exit
diff -Naurp pcp-4.1.0-orig/qa/1350.out pcp-4.1.0/qa/1350.out
--- pcp-4.1.0-orig/qa/1350.out	1970-01-01 10:00:00.000000000 +1000
+++ pcp-4.1.0/qa/1350.out	2018-10-09 15:57:39.403661320 +1100
@@ -0,0 +1,2 @@
+QA output created by 1350
+PASS, proc.psinfo.nvctxsw is nonzero
diff -Naurp pcp-4.1.0-orig/qa/group pcp-4.1.0/qa/group
--- pcp-4.1.0-orig/qa/group	2018-06-15 11:45:36.000000000 +1000
+++ pcp-4.1.0/qa/group	2018-10-09 15:57:39.403661320 +1100
@@ -1556,6 +1556,7 @@ BAD
 1342 pmda.prometheus local python
 1343 pmmgr local
 1344 pmmgr local
+1350 pmda.proc local
 1351 pmcd pmda.mmv libpcp libpcp_pmda local
 1354 pmval archive local
 1359 libpcp local
diff -Naurp pcp-4.1.0-orig/src/pmdas/linux_proc/proc_pid.c pcp-4.1.0/src/pmdas/linux_proc/proc_pid.c
--- pcp-4.1.0-orig/src/pmdas/linux_proc/proc_pid.c	2018-05-19 12:47:33.000000000 +1000
+++ pcp-4.1.0/src/pmdas/linux_proc/proc_pid.c	2018-10-09 15:57:39.402661334 +1100
@@ -1175,6 +1175,40 @@ maperr(void)
     return sts;
 }
 
+static int
+read_proc_entry(int fd, int *lenp, char **bufp)
+{
+    int sts = 0;
+    int n, len = 0;
+    char *p = *bufp;
+    char buf[1024];
+
+    for (len=0;;) {
+	if ((n = read(fd, buf, sizeof(buf))) <= 0)
+	    break;
+	len += n;
+	if (*lenp < len) {
+	    *lenp = len;
+	    *bufp = (char *)realloc(*bufp, len+1);
+	    p = *bufp + len - n;
+	}
+	memcpy(p, buf, n);
+	p += n;
+    }
+
+    if (len > 0)
+    	*p = '\0';
+    else {
+	/* invalid read */
+	if (n < 0)
+	    sts = maperr();
+    	else if (n == 0)
+	    sts = -ENODATA;
+    }
+
+    return sts;
+}
+
 /*
  * fetch a proc/<pid>/stat entry for pid
  */
@@ -1183,10 +1217,8 @@ fetch_proc_pid_stat(int id, proc_pid_t *
 {
     __pmHashNode	*node = __pmHashSearch(id, &proc_pid->pidhash);
     proc_pid_entry_t	*ep = node ? (proc_pid_entry_t *)node->data : NULL;
-    char		buf[1024];
     char		*p;
-    int			fd, n;
-    ssize_t		nread;
+    int			fd;
 
     *sts = 0;
     if (!ep)
@@ -1197,18 +1229,8 @@ fetch_proc_pid_stat(int id, proc_pid_t *
 	    ep->stat_buf[0] = '\0';
 	if ((fd = proc_open("stat", ep)) < 0)
 	    *sts = maperr();
-	else if ((n = read(fd, buf, sizeof(buf))) < 0)
-	    *sts = maperr();
-	else if (n == 0)
-	    *sts = -ENODATA;
-	else {
-	    if (ep->stat_buflen <= n) {
-		ep->stat_buflen = n;
-		ep->stat_buf = (char *)realloc(ep->stat_buf, n);
-	    }
-	    memcpy(ep->stat_buf, buf, n);
-	    ep->stat_buf[n-1] = '\0';
-	}
+	else
+	    *sts = read_proc_entry(fd, &ep->stat_buflen, &ep->stat_buf);
 	if (fd >= 0)
 	    close(fd);
 	ep->flags |= PROC_PID_FLAG_STAT_FETCHED;
@@ -1219,24 +1241,8 @@ fetch_proc_pid_stat(int id, proc_pid_t *
 	    ep->wchan_buf[0] = '\0';
 	if ((fd = proc_open("wchan", ep)) < 0)
 	    ; /* ignore failure here, backwards compat */
-	else if ((n = read(fd, buf, sizeof(buf)-1)) < 0)
-	    *sts = maperr();
-	else if (n == 0)
-	    /* wchan is empty, nothing to add here */
-	    ;
-	else {
-	    n++;	/* no terminating null (from kernel) */
-	    if (ep->wchan_buflen <= n) {
-		ep->wchan_buflen = n;
-		ep->wchan_buf = (char *)realloc(ep->wchan_buf, n);
-	    }
-	    if (ep->wchan_buf) {
-		memcpy(ep->wchan_buf, buf, n-1);
-		ep->wchan_buf[n-1] = '\0';
-	    } else {
-		ep->wchan_buflen = 0;
-	    }
-	}
+	else
+	    *sts = read_proc_entry(fd, &ep->wchan_buflen, &ep->wchan_buf);
 	if (fd >= 0)
 	    close(fd);
 	ep->flags |= PROC_PID_FLAG_WCHAN_FETCHED;
@@ -1246,22 +1252,17 @@ fetch_proc_pid_stat(int id, proc_pid_t *
 	if (ep->environ_buflen > 0)
 	    ep->environ_buf[0] = '\0';
 	if ((fd = proc_open("environ", ep)) >= 0) {
-	    nread = 0;
-	    while ((n = read(fd, buf, sizeof(buf))) > 0) {
-		if ((nread + n) >= ep->environ_buflen) {
-		    ep->environ_buflen = nread + n + 1;
-		    ep->environ_buf = realloc(ep->environ_buf, ep->environ_buflen);
-		}
-
+	    *sts = read_proc_entry(fd, &ep->environ_buflen, &ep->environ_buf);
+	    if (*sts == 0) {
 		/* Replace nulls with spaces */
-		for (p = memchr(buf, '\0', n); p; p = memchr(p, '\0', buf+n-p))
-		    *p = ' ';
-
-		memcpy(&ep->environ_buf[nread], buf, n);
-		nread += n;
+		if (ep->environ_buf) {
+		    for (p=ep->environ_buf; p < ep->environ_buf + ep->environ_buflen; p++) {
+			if (*p == '\0')
+			    *p = ' ';
+		    }
+		    ep->environ_buf[ep->environ_buflen-1] = '\0';
+		}
 	    }
-	    if (ep->environ_buf)
-		ep->environ_buf[nread] = '\0';
 	    else
 		ep->environ_buflen = 0;
 	    close(fd);
@@ -1313,31 +1314,14 @@ fetch_proc_pid_status(int id, proc_pid_t
 
     if (!(ep->flags & PROC_PID_FLAG_STATUS_FETCHED)) {
 	int	fd;
-	int	n;
-	char	buf[1024];
 	char	*curline;
 
 	if (ep->status_buflen > 0)
 	    ep->status_buf[0] = '\0';
 	if ((fd = proc_open("status", ep)) < 0)
 	    *sts = maperr();
-	else if ((n = read(fd, buf, sizeof(buf))) < 0)
-	    *sts = maperr();
-	else if (n == 0)
-	    *sts = -ENODATA;
-	else {
-	    if (ep->status_buflen < n) {
-		ep->status_buflen = n;
-		ep->status_buf = (char *)realloc(ep->status_buf, n);
-	    }
-	    if (ep->status_buf == NULL) {
-		ep->status_buflen = 0;
-		*sts = -ENODATA;
-	    } else {
-		memcpy(ep->status_buf, buf, n);
-		ep->status_buf[n-1] = '\0';
-	    }
-	}
+	else
+	    *sts = read_proc_entry(fd, &ep->status_buflen, &ep->status_buf);
 
 	if (*sts == 0) {
 	    /* assign pointers to individual lines in buffer */
@@ -1524,30 +1508,14 @@ fetch_proc_pid_statm(int id, proc_pid_t
     	return NULL;
 
     if (!(ep->flags & PROC_PID_FLAG_STATM_FETCHED)) {
-	char buf[1024];
-	int fd, n;
+	int fd;
 
 	if (ep->statm_buflen > 0)
 	    ep->statm_buf[0] = '\0';
 	if ((fd = proc_open("statm", ep)) < 0)
 	    *sts = maperr();
-	else if ((n = read(fd, buf, sizeof(buf))) < 0)
-	    *sts = maperr();
-	else if (n == 0)
-	    *sts = -ENODATA;
-	else {
-	    if (ep->statm_buflen <= n) {
-		ep->statm_buflen = n;
-		ep->statm_buf = (char *)realloc(ep->statm_buf, n);
-	    }
-	    if (ep->statm_buf) {
-		memcpy(ep->statm_buf, buf, n);
-		ep->statm_buf[n-1] = '\0';
-	    } else {
-		ep->statm_buflen = 0;
-	    }
-	}
-
+	else
+	    *sts = read_proc_entry(fd, &ep->statm_buflen, &ep->statm_buf);
 	if (fd >= 0)
 	    close(fd);
 	ep->flags |= PROC_PID_FLAG_STATM_FETCHED;
@@ -1567,7 +1535,6 @@ fetch_proc_pid_maps(int id, proc_pid_t *
 {
     __pmHashNode	*node = __pmHashSearch(id, &proc_pid->pidhash);
     proc_pid_entry_t	*ep = node ? (proc_pid_entry_t *)node->data : NULL;
-    char		*maps_bufptr = NULL;
     int			fd;
 
     *sts = 0;
@@ -1580,18 +1547,7 @@ fetch_proc_pid_maps(int id, proc_pid_t *
 	if ((fd = proc_open("maps", ep)) < 0)
 	    *sts = maperr();
 	else {
-	    char buf[1024];
-	    int n, len = 0;
-
-	    while ((n = read(fd, buf, sizeof(buf))) > 0) {
-		len += n;
-		if (ep->maps_buflen <= len) {
-		    ep->maps_buflen = len + 1;
-		    ep->maps_buf = (char *)realloc(ep->maps_buf, ep->maps_buflen);
-		}
-		maps_bufptr = ep->maps_buf + len - n;
-		memcpy(maps_bufptr, buf, n);
-	    }
+	    *sts = read_proc_entry(fd, &ep->maps_buflen, &ep->maps_buf);
 	    /* If there are no maps, make maps_buf a zero length string. */
 	    if (ep->maps_buflen == 0) {
 		ep->maps_buflen = 1;
@@ -1625,30 +1581,14 @@ fetch_proc_pid_schedstat(int id, proc_pi
 	return NULL;
 
     if (!(ep->flags & PROC_PID_FLAG_SCHEDSTAT_FETCHED)) {
-	int		fd, n;
-	char		buf[1024];
+	int		fd;
 
 	if (ep->schedstat_buflen > 0)
 	    ep->schedstat_buf[0] = '\0';
 	if ((fd = proc_open("schedstat", ep)) < 0)
 	    *sts = maperr();
-	else if ((n = read(fd, buf, sizeof(buf))) < 0)
-	    *sts = maperr();
-	else if (n == 0)
-	    *sts = -ENODATA;
-	else {
-	    if (ep->schedstat_buflen <= n) {
-		ep->schedstat_buflen = n;
-		ep->schedstat_buf = (char *)realloc(ep->schedstat_buf, n);
-	    }
-	    if (ep->schedstat_buf) {
-		memcpy(ep->schedstat_buf, buf, n);
-		ep->schedstat_buf[n-1] = '\0';
-	    } else {
-		ep->schedstat_buflen = 0;
-	    }
-	}
-
+	else
+	    *sts = read_proc_entry(fd, &ep->schedstat_buflen, &ep->schedstat_buf);
 	if (fd >= 0)
 	    close(fd);
 	ep->flags |= PROC_PID_FLAG_SCHEDSTAT_FETCHED;
@@ -1677,31 +1617,15 @@ fetch_proc_pid_io(int id, proc_pid_t *pr
 	return NULL;
 
     if (!(ep->flags & PROC_PID_FLAG_IO_FETCHED)) {
-	int	fd, n;
-	char	buf[1024];
+	int	fd;
 	char	*curline;
 
 	if (ep->io_buflen > 0)
 	    ep->io_buf[0] = '\0';
 	if ((fd = proc_open("io", ep)) < 0)
 	    *sts = maperr();
-	else if ((n = read(fd, buf, sizeof(buf))) < 0)
-	    *sts = maperr();
-	else if (n == 0)
-	    *sts = -ENODATA;
-	else {
-	    if (ep->io_buflen < n) {
-		ep->io_buflen = n;
-		ep->io_buf = (char *)realloc(ep->io_buf, n);
-	    }
-	    if (ep->io_buf) {
-		memcpy(ep->io_buf, buf, n);
-		ep->io_buf[n-1] = '\0';
-	    } else {
-		ep->io_buflen = 0;
-		*sts = -ENODATA;
-	    }
-	}
+	else
+	    *sts = read_proc_entry(fd, &ep->io_buflen, &ep->io_buf);
 
 	if (*sts == 0) {
 	    /* assign pointers to individual lines in buffer */