rewrite linux diskio code

Instead of using a hardcoded maximum number of slots for
stats of different disks, use a linked list. Also since the algorithm to
update each device's counters is the same for updating the totals, share
equal code, which in my eyes not only saves a bunch of LoC, but also
drastically increases readability.

src/common.c

@@ -33,6 +33,7 @@
 #include <errno.h>
 #include <sys/time.h>
 #include <pthread.h>
+#include "diskio.h"
 
 /* check for OS and include appropriate headers */
 #if defined(__linux__)

src/common.h

@@ -7,7 +7,6 @@
 #include <sys/socket.h>
 
 int check_mount(char *s);
-void update_diskio(void);
 void prepare_update(void);
 void update_uptime(void);
 void update_meminfo(void);
@@ -83,7 +82,6 @@ double get_sysfs_info(int *fd, int arg, char *devtype, char *type);
 
 void get_adt746x_cpu(char *, size_t);
 void get_adt746x_fan(char *, size_t);
-unsigned int get_diskio(void);
 
 int open_acpi_temperature(const char *name);
 double get_acpi_temperature(int fd);

src/diskio.c

@@ -48,79 +48,91 @@
 #define NBD_MAJOR 43
 #endif
 
-static struct diskio_stat diskio_stats_[MAX_DISKIO_STATS];
-struct diskio_stat *diskio_stats = diskio_stats_;
+/* this is the root of all per disk stats,
+ * also containing the totals. */
+static struct diskio_stat stats = {
+	.next = NULL,
+	.current = 0,
+	.current_read = 0,
+	.current_write = 0,
+	.last = UINT_MAX,
+	.last_read = UINT_MAX,
+	.last_write = UINT_MAX,
+};
 
 void clear_diskio_stats(void)
 {
-	unsigned i;
-	for(i = 1; i < MAX_DISKIO_STATS; i++) {
-		if (diskio_stats[i].dev) {
-			free(diskio_stats[i].dev);
-			diskio_stats[i].dev = 0;
-		}
+	struct diskio_stat *cur;
+	while (stats.next) {
+		cur = stats.next;
+		stats.next = stats.next->next;
+		free(cur);
 	}
 }
 
 struct diskio_stat *prepare_diskio_stat(const char *s)
 {
-	struct diskio_stat *new = 0;
-	unsigned i;
+	struct diskio_stat *cur = &stats;
 
 	if (!s)
-		return &diskio_stats[0];
+		return &stats;
 
-	/* lookup existing or get new */
-	for (i = 1; i < MAX_DISKIO_STATS; i++) {
-		if (diskio_stats[i].dev) {
-			if (strcmp(diskio_stats[i].dev, s) == 0) {
-				return &diskio_stats[i];
-			}
-		} else {
-			new = &diskio_stats[i];
-			break;
-		}
+	/* lookup existing */
+	while (cur->next) {
+		cur = cur->next;
+		if (!strcmp(cur->dev, s))
+			return cur;
 	}
-	/* new dev */
-	if (!new) {
-		ERR("too many diskio stats");
-		return 0;
+
+	/* no existing found, make a new one */
+	cur->next = malloc(sizeof(struct diskio_stat));
+	cur = cur->next;
+	memset(cur, 0, sizeof(struct diskio_stat));
+	cur->dev = strndup(s, text_buffer_size);
+	cur->last = UINT_MAX;
+	cur->last_read = UINT_MAX;
+	cur->last_write = UINT_MAX;
+	return cur;
+}
+
+static void update_diskio_values(struct diskio_stat *ds,
+		unsigned int reads, unsigned int writes)
+{
+	if (reads < ds->last_read || writes < ds->last_write) {
+		/* counter overflow or reset - rebase to sane values */
+		ds->last = 0;
+		ds->last_read = 0;
+		ds->last_write = 0;
 	}
-	if (new->dev) {
-		free(new->dev);
-		new->dev = 0;
-	}
-	new->dev = strndup(s, text_buffer_size);
-	new->current = 0;
-	new->current_read = 0;
-	new ->current_write = 0;
-	new->last = UINT_MAX;
-	new->last_read = UINT_MAX;
-	new->last_write = UINT_MAX;
-	return new;
+	/* since the values in /proc/diskstats are absolute, we have to substract
+	 * our last reading. The numbers stand for "sectors read", and we therefore
+	 * have to divide by two to get KB */
+	ds->current_read = (reads - ds->last_read) / 2;
+	ds->current_write = (writes - ds->last_write) / 2;
+	ds->current = ds->current_read + ds->current_write;
+
+	ds->last_read = reads;
+	ds->last_write = writes;
+	ds->last = ds->last_read + ds->last_write;
 }
 
 void update_diskio(void)
 {
-	static unsigned int last = UINT_MAX;
-	static unsigned int last_read = UINT_MAX;
-	static unsigned int last_write = UINT_MAX;
 	FILE *fp;
 	static int rep = 0;
 
+	struct diskio_stat *cur;
 	char buf[512], devbuf[64];
-	int i;
 	unsigned int major, minor;
-	unsigned int current = 0;
-	unsigned int current_read = 0;
-	unsigned int current_write = 0;
-	unsigned int reads, writes = 0;
+	unsigned int reads, writes;
+	unsigned int total_reads, total_writes;
 	int col_count = 0;
-	int tot, tot_read, tot_write;
+
+	stats.current = 0;
+	stats.current_read = 0;
+	stats.current_write = 0;
 
 	if (!(fp = open_file("/proc/diskstats", &rep))) {
-
-		diskio_stats[0].current = 0;
 		return;
 	}
 
@@ -135,9 +147,8 @@ void update_diskio(void)
 		 * XXX: ignore devices which are part of a SW RAID (MD_MAJOR) */
 		if (col_count == 5 && major != LVM_BLK_MAJOR && major != NBD_MAJOR
 				&& major != RAMDISK_MAJOR && major != LOOP_MAJOR) {
-			current += reads + writes;
-			current_read += reads;
-			current_write += writes;
+			total_reads += reads;
+			total_writes += writes;
 		} else {
 			col_count = sscanf(buf, "%u %u %s %*u %u %*u %u",
 				&major, &minor, devbuf, &reads, &writes);
@@ -145,60 +156,14 @@ void update_diskio(void)
 				continue;
 			}
 		}
-		for (i = 1; i < MAX_DISKIO_STATS; i++) {
-			if (diskio_stats[i].dev && !strcmp(devbuf, diskio_stats[i].dev)) {
-				diskio_stats[i].current =
-					(reads + writes - diskio_stats[i].last) / 2;
-				diskio_stats[i].current_read =
-					(reads - diskio_stats[i].last_read) / 2;
-				diskio_stats[i].current_write =
-					(writes - diskio_stats[i].last_write) / 2;
-				if (reads + writes < diskio_stats[i].last) {
-					diskio_stats[i].current = 0;
-				}
-				if (reads < diskio_stats[i].last_read) {
-					diskio_stats[i].current_read = 0;
-					diskio_stats[i].current = diskio_stats[i].current_write;
-				}
-				if (writes < diskio_stats[i].last_write) {
-					diskio_stats[i].current_write = 0;
-					diskio_stats[i].current = diskio_stats[i].current_read;
-				}
-				diskio_stats[i].last = reads + writes;
-				diskio_stats[i].last_read = reads;
-				diskio_stats[i].last_write = writes;
-			}
-		}
+		cur = stats.next;
+		while (cur && strcmp(devbuf, cur->dev))
+			cur = cur->next;
+
+		if (cur)
+			update_diskio_values(cur, reads, writes);
 	}
-
-	/* since the values in /proc/diststats are absolute, we have to substract
-	 * our last reading. The numbers stand for "sectors read", and we therefore
-	 * have to divide by two to get KB */
-	tot = ((double) (current - last) / 2);
-	tot_read = ((double) (current_read - last_read) / 2);
-	tot_write = ((double) (current_write - last_write) / 2);
-
-	if (last_read > current_read) {
-		tot_read = 0;
-	}
-	if (last_write > current_write) {
-		tot_write = 0;
-	}
-
-	if (last > current) {
-		/* we hit this either if it's the very first time we run this, or
-		 * when /proc/diskstats overflows; while 0 is not correct, it's at
-		 * least not way off */
-		tot = 0;
-	}
-	last = current;
-	last_read = current_read;
-	last_write = current_write;
-
-	diskio_stats[0].current = tot;
-	diskio_stats[0].current_read = tot_read;
-	diskio_stats[0].current_write = tot_write;
-
+	update_diskio_values(&stats, total_reads, total_writes);
 	fclose(fp);
 }
 

src/diskio.h

@@ -30,16 +30,18 @@
 #define DISKIO_H_
 
 struct diskio_stat {
+	struct diskio_stat *next;
 	char *dev;
-	unsigned int current, current_read, current_write, last, last_read,
-		last_write;
+	unsigned int current;
+	unsigned int current_read;
+	unsigned int current_write;
+	unsigned int last;
+	unsigned int last_read;
+	unsigned int last_write;
 };
 
-#define MAX_DISKIO_STATS 64
-
-struct diskio_stat *diskio_stats;
-
 struct diskio_stat *prepare_diskio_stat(const char *s);
+void update_diskio(void);
 void clear_diskio_stats(void);
 
 #endif /* DISKIO_H_ */