cleanup various warnings and one static diagnostic

Signed-off-by: Tom St Denis <tom.stdenis@amd.com>

14 files changed, 96 insertions, 41 deletions

diff --git a/src/app/main.c b/src/app/main.c
index 41a9c11..2d29bbd 100644
--- a/src/app/main.c
+++ b/src/app/main.c
@@ -372,17 +372,27 @@ int main(int argc, char **argv)
 			if (!asic)
 				asic = get_asic();
 			if (options.follow) {
+				int r;
+
 				signal(SIGINT, sigint);
-				system("echo 1 > /sys/kernel/debug/tracing/events/amdgpu/amdgpu_mm_wreg/enable");
-				system("echo 1 > /sys/kernel/debug/tracing/events/amdgpu/amdgpu_mm_rreg/enable");
+				r = system("echo 1 > /sys/kernel/debug/tracing/events/amdgpu/amdgpu_mm_wreg/enable");
+				r |= system("echo 1 > /sys/kernel/debug/tracing/events/amdgpu/amdgpu_mm_rreg/enable");
+				if (r) {
+					fprintf(stderr, "[ERROR]: Could not enable mm tracers\n");
+					return EXIT_FAILURE;
+				}
 				req.tv_sec = 0;
 				req.tv_nsec = 1000000000/10; // 100ms
 				while (!quit) {
 					nanosleep(&req, NULL);
 					umr_scan_log(asic);
 				}
-				system("echo 0 > /sys/kernel/debug/tracing/events/amdgpu/amdgpu_mm_wreg/enable");
-				system("echo 0 > /sys/kernel/debug/tracing/events/amdgpu/amdgpu_mm_rreg/enable");
+				r = system("echo 0 > /sys/kernel/debug/tracing/events/amdgpu/amdgpu_mm_wreg/enable");
+				r |= system("echo 0 > /sys/kernel/debug/tracing/events/amdgpu/amdgpu_mm_rreg/enable");
+				if (r) {
+					fprintf(stderr, "[ERROR]: Could not diable mm tracers\n");
+					return EXIT_FAILURE;
+				}
 			} else {
 				umr_scan_log(asic);
 			}
@@ -493,7 +503,7 @@ int main(int argc, char **argv)
 				sscanf(argv[i+2], "%"SCNx32, &size);
 				do {
 					n = size > sizeof(buf) ? sizeof(buf) : size;
-					fread(buf, 1, n, stdin);
+					n = fread(buf, 1, n, stdin);
 					if (umr_write_vram(asic, vmid, address, n, buf))
 						return EXIT_FAILURE;
 					size -= n;
diff --git a/src/app/set_bit.c b/src/app/set_bit.c
index e109069..19293b7 100644
--- a/src/app/set_bit.c
+++ b/src/app/set_bit.c
@@ -74,7 +74,8 @@ int umr_set_register_bit(struct umr_asic *asic, char *regpath, char *regvalue)
 										addr = 0;
 
 									lseek(fd, addr | (asic->blocks[i]->regs[j].addr*scale), SEEK_SET);
-									read(fd, &copy, 4);
+									if (read(fd, &copy, 4) != 4)
+										return -1;
 
 									// read-modify-write value back
 									copy &= ~mask;
@@ -82,7 +83,8 @@ int umr_set_register_bit(struct umr_asic *asic, char *regpath, char *regvalue)
 									copy |= value;
 
 									lseek(fd, addr | (asic->blocks[i]->regs[j].addr<<2), SEEK_SET);
-									write(fd, &copy, 4);
+									if (write(fd, &copy, 4) != 4)
+										return -1;
 									if (!asic->options.quiet) printf("%s <= 0x%08lx\n", regpath, (unsigned long)copy);
 
 									if (asic->blocks[i]->release) {
diff --git a/src/app/set_reg.c b/src/app/set_reg.c
index 1df68f1..b6e285d 100644
--- a/src/app/set_reg.c
+++ b/src/app/set_reg.c
@@ -71,7 +71,8 @@ int umr_set_register(struct umr_asic *asic, char *regpath, char *regvalue)
 								addr = 0;
 
 							lseek(fd, addr | (asic->blocks[i]->regs[j].addr*scale), SEEK_SET);
-							write(fd, &value, 4);
+							if (write(fd, &value, 4) != 4)
+								return -1;
 
 							if (asic->blocks[i]->release) {
 								if (asic->blocks[i]->release(asic)) {
diff --git a/src/app/top.c b/src/app/top.c
index 7109e12..d130319 100644
--- a/src/app/top.c
+++ b/src/app/top.c
@@ -628,7 +628,8 @@ static void parse_bits(struct umr_asic *asic, uint32_t addr, struct umr_bitfield
 			value = asic->pci.mem[addr>>2];
 		} else {
 			lseek(asic->fd.mmio, addr | addr_mask, SEEK_SET);
-			read(asic->fd.mmio, &value, 4);
+			if (read(asic->fd.mmio, &value, 4) != 4)
+				value = 0;
 		}
 		for (j = 0; bits[j].regname; j++)
 			if (bits[j].start != 255) {
@@ -724,7 +725,8 @@ static void parse_iov(struct umr_asic *asic, uint32_t addr, struct umr_bitfield
 			value = asic->pci.mem[addr>>2];
 		} else {
 			lseek(asic->fd.mmio, addr | addr_mask, SEEK_SET);
-			read(asic->fd.mmio, &value, 4);
+			if (read(asic->fd.mmio, &value, 4) != 4)
+				value = 0;
 		}
 		for (j = 0; bits[j].regname; j++)
 			if (bits[j].start != 255) {
@@ -844,28 +846,34 @@ void load_options(void)
 {
 	FILE *f;
 	char path[512];
+	int r;
 
 	memset(&top_options, 0, sizeof(top_options));
 
 	sprintf(path, "%s/.umrtop", getenv("HOME"));
 	f = fopen(path, "r");
 	if (f) {
-		fscanf(f, "%d\n", &top_options.wide);
-		fscanf(f, "%d\n", &top_options.vram);
-		fscanf(f, "%d\n", &top_options.high_precision);
-		fscanf(f, "%d\n", &top_options.high_frequency);
-		fscanf(f, "%d\n", &top_options.all);
-		fscanf(f, "%d\n", &top_options.drm);
-		fscanf(f, "%d\n", &top_options.vi.ta);
-		fscanf(f, "%d\n", &top_options.vi.vgt);
-		fscanf(f, "%d\n", &top_options.vi.uvd);
-		fscanf(f, "%d\n", &top_options.vi.vce);
-		fscanf(f, "%d\n", &top_options.vi.gfxpwr);
-		fscanf(f, "%d\n", &top_options.vi.grbm);
-		fscanf(f, "%d\n", &top_options.vi.memory_hub);
-		fscanf(f, "%d\n", &top_options.vi.sdma);
-		fscanf(f, "%d\n", &top_options.vi.sensors);
+		r = 1;
+		r &= fscanf(f, "%d\n", &top_options.wide);
+		r &= fscanf(f, "%d\n", &top_options.vram);
+		r &= fscanf(f, "%d\n", &top_options.high_precision);
+		r &= fscanf(f, "%d\n", &top_options.high_frequency);
+		r &= fscanf(f, "%d\n", &top_options.all);
+		r &= fscanf(f, "%d\n", &top_options.drm);
+		r &= fscanf(f, "%d\n", &top_options.vi.ta);
+		r &= fscanf(f, "%d\n", &top_options.vi.vgt);
+		r &= fscanf(f, "%d\n", &top_options.vi.uvd);
+		r &= fscanf(f, "%d\n", &top_options.vi.vce);
+		r &= fscanf(f, "%d\n", &top_options.vi.gfxpwr);
+		r &= fscanf(f, "%d\n", &top_options.vi.grbm);
+		r &= fscanf(f, "%d\n", &top_options.vi.memory_hub);
+		r &= fscanf(f, "%d\n", &top_options.vi.sdma);
+		r &= fscanf(f, "%d\n", &top_options.vi.sensors);
 		fclose(f);
+		if (!r) {
+			fprintf(stderr, "[WARNING]: --top option missing from configuration file please save configuration before quitting\n");
+			sleep(2);
+		}
 	} else {
 		// add some defaults to not be so boring
 		top_options.vi.grbm = 1;
@@ -1097,7 +1105,8 @@ int get_active_vf(struct umr_asic *asic, uint32_t addr)
 			value = asic->pci.mem[addr>>2];
 		} else {
 			lseek(asic->fd.mmio, addr, SEEK_SET);
-			read(asic->fd.mmio, &value, 4);
+			if (read(asic->fd.mmio, &value, 4) != 4)
+				return 0;
 		}
 		value &= 0xF;
 	}
diff --git a/src/lib/discover.c b/src/lib/discover.c
index 78d572d..819898f 100644
--- a/src/lib/discover.c
+++ b/src/lib/discover.c
@@ -194,8 +194,12 @@ struct umr_asic *umr_discover_asic(struct umr_options *options)
 		}
 		return NULL;
 	} else if (f) {
-		fscanf(f, "%*s %s", name);
+		int r;
+
+		r = fscanf(f, "%*s %s", name);
 		fclose(f);
+		if (r != 1)
+			return NULL;
 
 		// strip off dev= for kernels > 4.7
 		if (strstr(name, "dev="))
diff --git a/src/lib/discover_by_did.c b/src/lib/discover_by_did.c
index 41c9dd2..92ee10a 100644
--- a/src/lib/discover_by_did.c
+++ b/src/lib/discover_by_did.c
@@ -255,9 +255,14 @@ static int find_first_did(long did, long start_instance)
 		f = fopen(name, "r");
 		if (f) {
 			unsigned tmp_did;
-			fscanf(f, "%*s %s", name);
+			int r;
+
+			r = fscanf(f, "%*s %s", name);
 			fclose(f);
 
+			if (r != 1)
+				return -1;
+
 			// strip off dev= for kernels > 4.7
 			if (strstr(name, "dev="))
 				memmove(name, name+4, strlen(name)-3);
@@ -265,8 +270,7 @@ static int find_first_did(long did, long start_instance)
 			snprintf(device, sizeof(device)-1, "/sys/bus/pci/devices/%s/device", name);
 			f2 = fopen(device, "r");
 			if (f2) {
-				fscanf(f, "0x%04x", &tmp_did);
-				if (tmp_did == did) {
+				if (fscanf(f, "0x%04x", &tmp_did) == 1 && tmp_did == did) {
 					fclose(f2);
 					return x;
 				}
diff --git a/src/lib/find_reg.c b/src/lib/find_reg.c
index 898858e..9661060 100644
--- a/src/lib/find_reg.c
+++ b/src/lib/find_reg.c
@@ -84,6 +84,9 @@ struct umr_reg *umr_find_reg_by_addr(struct umr_asic *asic, uint64_t addr, struc
 {
 	int i, j;
 
+	if (ip)
+		*ip = NULL;
+
 	if (asic->mmio_accel.reglist && asic->mmio_accel.reglist[addr]) {
 		if (ip && asic->mmio_accel.iplist)
 			*ip = asic->mmio_accel.iplist[addr];
diff --git a/src/lib/read_vram.c b/src/lib/read_vram.c
index 57a7ee8..614d20c 100644
--- a/src/lib/read_vram.c
+++ b/src/lib/read_vram.c
@@ -31,7 +31,10 @@ static uint64_t dma_to_phys(struct umr_asic *asic, uint64_t dma_addr)
 	uint64_t phys;
 	if (asic->fd.iova >= 0) {
 		lseek(asic->fd.iova, dma_addr & ~0xFFFULL, SEEK_SET);
-		read(asic->fd.iova, &phys, 8);
+		if (read(asic->fd.iova, &phys, 8) != 8) {
+			fprintf(stderr, "[ERROR]: Could not read from debugfs iova file for address %" PRIx64 "\n", dma_addr);
+			return 0;
+		}
 	} else {
 		phys = dma_addr;
 	}
diff --git a/src/lib/ring_decode.c b/src/lib/ring_decode.c
index 156291d..7aac4f7 100644
--- a/src/lib/ring_decode.c
+++ b/src/lib/ring_decode.c
@@ -717,8 +717,10 @@ static void print_decode_pm4_pkt3(struct umr_asic *asic, struct umr_ring_decoder
 						// decode additional words
 						printf("DATA: %s0x%08lx%s", BLUE, (unsigned long)ib, RST);
 						break;
+					} else {
+						printf("Invalid word for opcode 0x%02lx", (unsigned long)decoder->pm4.cur_opcode);
 					}
-					// fall through to invalid
+					break;
 				default: printf("Invalid word for opcode 0x%02lx", (unsigned long)decoder->pm4.cur_opcode);
 			}
 			break;
diff --git a/src/lib/scan_config.c b/src/lib/scan_config.c
index 9574834..10f1f74 100644
--- a/src/lib/scan_config.c
+++ b/src/lib/scan_config.c
@@ -100,8 +100,8 @@ int umr_scan_config(struct umr_asic *asic)
 	snprintf(fname, sizeof(fname)-1, "/sys/bus/pci/devices/%s/vbios_version", asic->options.pci.name);
 	f = fopen(fname, "r");
 	if (f) {
-		fgets(asic->config.vbios_version, sizeof(asic->config.vbios_version)-1, f);
-		asic->config.vbios_version[strlen(asic->config.vbios_version)-1] = 0; // remove newline...
+		if (fgets(asic->config.vbios_version, sizeof(asic->config.vbios_version)-1, f))
+			asic->config.vbios_version[strlen(asic->config.vbios_version)-1] = 0; // remove newline...
 		fclose(f);
 	}
 
@@ -126,8 +126,10 @@ gca_config:
 	f = fopen(fname, "rb");
 	if (!f)
 		return -1;
-	fread(data, 1, sizeof(data), f);
+	r = fread(data, 1, sizeof(data), f);
 	fclose(f);
+	if (r < 0)
+		return -1;
 
 	switch (data[0]) {
 		case 0: parse_rev0(asic, data, &r);
diff --git a/src/lib/umr_read_ring_data.c b/src/lib/umr_read_ring_data.c
index 8889f4b..778b471 100644
--- a/src/lib/umr_read_ring_data.c
+++ b/src/lib/umr_read_ring_data.c
@@ -39,6 +39,7 @@
 void *umr_read_ring_data(struct umr_asic *asic, char *ringname, uint32_t *ringsize)
 {
 	int fd;
+	uint32_t r;
 	void *ring_data;
 	char fname[128];
 
@@ -59,7 +60,11 @@ void *umr_read_ring_data(struct umr_asic *asic, char *ringname, uint32_t *ringsi
 		fprintf(stderr, "[ERROR]: Out of memory\n");
 		return NULL;
 	}
-	read(fd, ring_data, *ringsize + 12);
+	r = read(fd, ring_data, *ringsize + 12);
 	close(fd);
+	if (r != *ringsize + 12) {
+		free(ring_data);
+		return NULL;
+	}
 	return ring_data;
 }
diff --git a/src/lib/update.c b/src/lib/update.c
index 96ae39f..a51614f 100644
--- a/src/lib/update.c
+++ b/src/lib/update.c
@@ -403,7 +403,7 @@ int umr_update(struct umr_asic *asic, char *script)
 	int fd;
 	void *smem;
 	char *sdata;
-	unsigned len;
+	unsigned len, r;
 
 	fd = open(script, O_RDWR);
 	if (fd < 0) {
@@ -418,8 +418,12 @@ int umr_update(struct umr_asic *asic, char *script)
 		fprintf(stderr, "[ERROR]: Out of memory...\n");
 		return -1;
 	}
-	read(fd, smem, len);
+	r = read(fd, smem, len);
 	close(fd);
+	if (r != len) {
+		free(smem);
+		return 0;
+	}
 
 	// parse script
 	while (*sdata) {
diff --git a/src/lib/wave_status.c b/src/lib/wave_status.c
index 0ec9a5a..1d48fb3 100644
--- a/src/lib/wave_status.c
+++ b/src/lib/wave_status.c
@@ -102,6 +102,7 @@ static int read_wave_status_via_mmio(struct umr_asic *asic, uint32_t simd, uint3
 static int umr_get_wave_status_vi(struct umr_asic *asic, unsigned se, unsigned sh, unsigned cu, unsigned simd, unsigned wave, struct umr_wave_status *ws)
 {
 	uint32_t x, value, buf[32];
+	int r;
 
 	memset(buf, 0, sizeof buf);
 
@@ -113,7 +114,9 @@ static int umr_get_wave_status_vi(struct umr_asic *asic, unsigned se, unsigned s
 			((uint64_t)cu << 23) |
 			((uint64_t)wave << 31) |
 			((uint64_t)simd << 37), SEEK_SET);
-		read(asic->fd.wave, &buf, 32*4);
+		r = read(asic->fd.wave, &buf, 32*4);
+		if (r <= 0)
+			return -1;
 	} else {
 		int n = 0;
 		umr_grbm_select_index(asic, se, sh, cu);
@@ -207,6 +210,7 @@ static int umr_get_wave_status_vi(struct umr_asic *asic, unsigned se, unsigned s
 static int umr_get_wave_status_ai(struct umr_asic *asic, unsigned se, unsigned sh, unsigned cu, unsigned simd, unsigned wave, struct umr_wave_status *ws)
 {
 	uint32_t x, value, buf[32];
+	int r;
 
 	memset(buf, 0, sizeof buf);
 
@@ -218,7 +222,9 @@ static int umr_get_wave_status_ai(struct umr_asic *asic, unsigned se, unsigned s
 			((uint64_t)cu << 23) |
 			((uint64_t)wave << 31) |
 			((uint64_t)simd << 37), SEEK_SET);
-		read(asic->fd.wave, &buf, 32*4);
+		r = read(asic->fd.wave, &buf, 32*4);
+		if (r < 0)
+			return -1;
 	} else {
 		int n = 0;
 		umr_grbm_select_index(asic, se, sh, cu);
diff --git a/src/umr.h b/src/umr.h
index 2e40e0b..37fc720 100644
--- a/src/umr.h
+++ b/src/umr.h
@@ -216,7 +216,7 @@ struct umr_options {
 		hub_name[32],
 		ring_name[32];
 	struct {
-		int domain,
+		unsigned domain,
 		    bus,
 		    slot,
 		    func;