Rewrite hddtemp support for better scaling

Instead of connecting once for each object, have a central update
routine (limiting support to only a single hddtemp daemon to connect
to).

ChangeLog

@@ -1,3 +1,6 @@
+2009-09-06
+	* Rewrite hddtemp support for better scaling.
+
 2009-08-10
 	* Add day and date data_type to $weather_forecast
 

doc/config_settings.xml

@@ -276,6 +276,26 @@
         For other position related stuff, see 'alignment'. 
         <para /></listitem>
     </varlistentry>
+    <varlistentry>
+        <term>
+            <command>
+                <option>hddtemp_host</option>
+            </command>
+        </term>
+	<listitem>Hostname to connect to for hddtemp objects. Defaults
+		to "127.0.0.1".
+        <para /></listitem>
+    </varlistentry>
+    <varlistentry>
+        <term>
+            <command>
+                <option>hddtemp_port</option>
+            </command>
+        </term>
+	<listitem>Port to use for hddtemp connections. Defaults to
+		7634.
+        <para /></listitem>
+    </varlistentry>
     <varlistentry>
         <term>
             <command>

doc/variables.xml

@@ -1251,12 +1251,13 @@
             <command>
                 <option>hddtemp</option>
             </command>
-            <option>dev, (host,(port))</option>
+            <option>(dev)</option>
         </term>
         <listitem>Displays temperature of a selected hard disk
-        drive as reported by the hddtemp daemon running on
+		drive as reported by the hddtemp daemon. Use hddtemp_host
-        host:port. Default host is 127.0.0.1, default port is 7634.
+		and hddtemp_port to specify a host and port for all hddtemp
-        
+		objects. If no dev parameter is given, the first disk returned
+		by the hddtemp daemon is used.
         <para /></listitem>
     </varlistentry>
     <varlistentry>

src/common.c

@@ -530,6 +530,11 @@ void update_stuff(void)
 		update_apcupsd();
 	}
 #endif
+#ifdef HDDTEMP
+	if (NEED(INFO_HDDTEMP)) {
+		update_hddtemp();
+	}
+#endif
 }
 
 /* Ohkie to return negative values for temperatures */

src/conky.c

@@ -1966,29 +1966,14 @@ static void generate_text_internal(char *p, int p_max_size,
 #endif /* HAVE_LUA */
 #ifdef HDDTEMP
 			OBJ(hddtemp) {
-				char *endptr, unit;
+				short val;
-				long val;
+				char unit;
-				if (obj->data.hddtemp.update_time < current_update_time - 30) {
+
-					if (obj->data.hddtemp.temp)
+				if (get_hddtemp_info(obj->data.s, &val, &unit)) {
-						free(obj->data.hddtemp.temp);
-					obj->data.hddtemp.temp = get_hddtemp_info(obj->data.hddtemp.dev,
-							obj->data.hddtemp.addr, obj->data.hddtemp.port);
-					obj->data.hddtemp.update_time = current_update_time;
-				}
-				if (!obj->data.hddtemp.temp) {
 					snprintf(p, p_max_size, "N/A");
 				} else {
-					val = strtol(obj->data.hddtemp.temp + 1, &endptr, 10);
+					temp_print(p, p_max_size, (double)val,
-					unit = obj->data.hddtemp.temp[0];
+							(unit == 'C' ? TEMP_CELSIUS : TEMP_FAHRENHEIT));
-
-					if (*endptr != '\0')
-						snprintf(p, p_max_size, "N/A");
-					else if (unit == 'C')
-						temp_print(p, p_max_size, (double)val, TEMP_CELSIUS);
-					else if (unit == 'F')
-						temp_print(p, p_max_size, (double)val, TEMP_FAHRENHEIT);
-					else
-						snprintf(p, p_max_size, "N/A");
 				}
 			}
 #endif
@@ -5798,6 +5783,14 @@ char load_config_file(const char *f)
 		CONF("format_human_readable") {
 			format_human_readable = string_to_bool(value);
 		}
+#ifdef HDDTEMP
+		CONF("hddtemp_host") {
+			set_hddtemp_host(value);
+		}
+		CONF("hddtemp_port") {
+			set_hddtemp_port(value);
+		}
+#endif /* HDDTEMP */
 		CONF("pad_percents") {
 			pad_percents = atoi(value);
 		}

src/conky.h

@@ -262,6 +262,9 @@ enum {
 #ifdef WEATHER
 	INFO_WEATHER = 33,
 #endif
+#ifdef HDDTEMP
+	INFO_HDDTEMP = 34,
+#endif
 };
 
 /* get_battery_stuff() item selector

src/core.c

@@ -1989,15 +1989,9 @@ struct text_object *construct_text_object(const char *s, const char *arg, long
 #endif /* X11 */
 #endif /* HAVE_LUA */
 #ifdef HDDTEMP
-	END OBJ(hddtemp, 0)
+	END OBJ(hddtemp, INFO_HDDTEMP)
-		if (scan_hddtemp(arg, &obj->data.hddtemp.dev,
+		if (arg)
-		                 &obj->data.hddtemp.addr, &obj->data.hddtemp.port)) {
+			obj->data.s = strndup(arg, text_buffer_size);
-			NORM_ERR("hddtemp needs arguments");
-			obj->type = OBJ_text;
-			obj->data.s = strndup("${hddtemp}", text_buffer_size);
-			obj->data.hddtemp.update_time = 0;
-		} else
-			obj->data.hddtemp.temp = NULL;
 #endif /* HDDTEMP */
 #ifdef TCP_PORT_MONITOR
 	END OBJ(tcp_portmon, INFO_TCP_PORT_MONITOR)
@@ -2672,10 +2666,11 @@ void free_text_objects(struct text_object *root, int internal)
 				break;
 #ifdef HDDTEMP
 			case OBJ_hddtemp:
-				free(data.hddtemp.dev);
+				if (data.s) {
-				free(data.hddtemp.addr);
+					free(data.s);
-				if (data.hddtemp.temp)
+					data.s = NULL;
-					free(data.hddtemp.temp);
+				}
+				free_hddtemp();
 				break;
 #endif /* HDDTEMP */
 			case OBJ_entropy_avail:

src/hddtemp.c

@@ -40,173 +40,217 @@
 #include <netinet/in.h>
 
 #define BUFLEN 512
-#define PORT 7634
+#define DEFAULT_PORT "7634"
+#define DEFAULT_HOST "127.0.0.1"
 
-char buf[BUFLEN];
+static char *hddtemp_host = NULL;
+static char *hddtemp_port = NULL;
 
-int scan_hddtemp(const char *arg, char **dev, char **addr, int *port)
+static struct hdd_info {
+	struct hdd_info *next;
+	char *dev;
+	short temp;
+	char unit;
+} hdd_info_head = {
+	.next = NULL,
+};
+
+void set_hddtemp_host(const char *host)
 {
-	char buf1[32], buf2[64];
+	if (hddtemp_host)
-	int n, ret;
+		free(hddtemp_host);
+	hddtemp_host = strdup(host);
+}
 
-	if (!arg)
+void set_hddtemp_port(const char *port)
-		return 1;
+{
+	if (hddtemp_port)
+		free(hddtemp_port);
+	hddtemp_port = strdup(port);
+}
 
-	if ((ret = sscanf(arg, "%31s %63s %d", buf1, buf2, &n)) < 1)
+static void __free_hddtemp_info(struct hdd_info *hdi)
-		return 1;
+{
+	if (hdi->next)
+		__free_hddtemp_info(hdi->next);
+	free(hdi->dev);
+	free(hdi);
+}
 
-	if (strncmp(buf1, "/dev/", 5)) {
+static void free_hddtemp_info(void)
-		strncpy(buf1 + 5, buf1, 32 - 5);
+{
-		strncpy(buf1, "/dev/", 5);
+	DBGP("free_hddtemp_info() called");
-	}
+	if (!hdd_info_head.next)
-	*dev = strndup(buf1, text_buffer_size);
+		return;
+	__free_hddtemp_info(hdd_info_head.next);
+	hdd_info_head.next = NULL;
+}
 
-	if (ret >= 2) {
+static void add_hddtemp_info(char *dev, short temp, char unit)
-		*addr = strndup(buf2, text_buffer_size);
+{
-	} else {
+	struct hdd_info *hdi = &hdd_info_head;
-		*addr = strndup("127.0.0.1", text_buffer_size);
+
+	DBGP("add_hddtemp_info(%s, %d, %c) being called", dev, temp, unit);
+	while (hdi->next)
+		hdi = hdi->next;
+
+	hdi->next = malloc(sizeof(struct hdd_info));
+	memset(hdi->next, 0, sizeof(struct hdd_info));
+	hdi->next->dev = strdup(dev);
+	hdi->next->temp = temp;
+	hdi->next->unit = unit;
+}
+
+static char *fetch_hddtemp_output(void)
+{
+	int sockfd;
+	const char *dst_host, *dst_port;
+	char *buf = NULL;
+	int buflen, offset = 0, rlen;
+	struct addrinfo hints, *result, *rp;
+	int i;
+
+	dst_host = hddtemp_host ? hddtemp_host : DEFAULT_HOST;
+	dst_port = hddtemp_port ? hddtemp_port : DEFAULT_PORT;
+
+	memset(&hints, 0, sizeof(hints));
+	hints.ai_family = AF_INET;	/* XXX: hddtemp has no ipv6 support (yet?) */
+	hints.ai_socktype = SOCK_STREAM;
+
+	if ((i = getaddrinfo(dst_host, dst_port, &hints, &result))) {
+		NORM_ERR("getaddrinfo(): %s", gai_strerror(i));
+		return NULL;
 	}
 
-	if (ret == 3) {
+	for (rp = result; rp; rp = rp->ai_next) {
-		*port = n;
+		sockfd = socket(rp->ai_family, rp->ai_socktype, rp->ai_protocol);
-	} else {
+		if (sockfd == -1)
-		*port = PORT;
+			continue;
+		if (connect(sockfd, rp->ai_addr, rp->ai_addrlen) != -1)
+			break;
+		close(sockfd);
+	}
+	if (!rp) {
+		NORM_ERR("could not connect to mpd host");
+		goto GET_OUT;
 	}
 
-	return 0;
+	buflen = 1024;
+	buf = malloc(buflen);
+	while ((rlen = recv(sockfd, buf + offset, buflen - offset - 1, 0)) > 0) {
+		offset += rlen;
+		if (buflen - offset < 1) {
+			buflen += 1024;
+			buf = realloc(buf, buflen);
+		}
+	}
+	if (rlen < 0)
+		perror("recv");
+
+	buf[offset] = '\0';
+
+	close(sockfd);
+GET_OUT:
+	freeaddrinfo(result);
+	return buf;
 }
 
 /* this is an iterator:
  * set line to NULL in consecutive calls to get the next field
- * returns "<dev><unit><val>" or NULL on error
+ * note that exhausing iteration is assumed - otherwise *saveptr
+ * is not being freed!
  */
-static char *read_hdd_val(const char *line)
+static int read_hdd_val(const char *line, char **dev, short *val, char *unit,
+		char **saveptr)
 {
-	static char line_s[512] = "\0";
+	char *line_s, *cval, *endptr;
 	static char *p = 0;
-	char *dev, *val, unit;
-	char *ret = NULL;
 
 	if (line) {
-		if (!snprintf(line_s, 512, "%s", line)) return ret;
+		*saveptr = strdup(line);
-		p = line_s;
+		p = *saveptr;
 	}
-	if (!(*line_s) || !p)
+	line_s = *saveptr;
-		return ret;
+
 	/* read the device */
-	dev = ++p;
+	*dev = ++p;
 	if (!(p = strchr(p, line_s[0])))
-		return ret;
+		goto out_fail;
 	*(p++) = '\0';
 	/* jump over the devname */
 	if (!(p = strchr(p, line_s[0])))
-		return ret;
+		goto out_fail;
 	/* read the value */
-	val = ++p;
+	cval = ++p;
 	if (!(p = strchr(p, line_s[0])))
-		return ret;
+		goto out_fail;
 	*(p++) = '\0';
-	unit = *(p++);
+	*unit = *(p++);
+	*val = strtol(cval, &endptr, 10);
+	if (*endptr)
+		goto out_fail;
+	
 	/* preset p for next call */
-	p = strchr(p + 1, line_s[0]);
+	p++;
 
-	if (dev && *dev && val && *val) {
+	return 0;
-		asprintf(&ret, "%s%c%s", dev, unit, val);
+out_fail:
-	}
+	free(*saveptr);
-	return ret;
+	return 1;
 }
 
-/* returns <unit><val> or NULL on error or N/A */
+void update_hddtemp(void) {
-char *get_hddtemp_info(char *dev, char *hostaddr, int port)
+	char *data, *dev, unit, *saveptr;
+	short val;
+	static double last_hddtemp_update = 0.0;
+
+	/* limit tcp connection overhead */
+	if (current_update_time - last_hddtemp_update < 5)
+		return;
+	last_hddtemp_update = current_update_time;
+
+	free_hddtemp_info();
+
+	if (!(data = fetch_hddtemp_output()))
+		return;
+
+	if (read_hdd_val(data, &dev, &val, &unit, &saveptr)) {
+		free(data);
+		return;
+	}
+	do {
+		add_hddtemp_info(dev, val, unit);
+	} while (!read_hdd_val(NULL, &dev, &val, &unit, &saveptr));
+	free(data);
+}
+
+void free_hddtemp(void)
 {
-	int sockfd = 0;
+	free_hddtemp_info();
-	struct hostent he, *he_res = 0;
+	if (hddtemp_host) {
-	int he_errno;
+		free(hddtemp_host);
-	char hostbuff[2048];
+		hddtemp_host = NULL;
-	struct sockaddr_in addr;
-	struct timeval tv;
-	fd_set rfds;
-	int len, i;
-	char *p, *r = NULL;
-
-	if ((sockfd = socket(AF_INET, SOCK_STREAM, 0)) == -1) {
-		perror("socket");
-		goto GET_OUT;
 	}
-
+	if (hddtemp_port) {
-#ifdef HAVE_GETHOSTBYNAME_R
+		free(hddtemp_port);
-	if (gethostbyname_r(hostaddr, &he, hostbuff,
+		hddtemp_port = NULL;
-	                    sizeof(hostbuff), &he_res, &he_errno)) {
-		NORM_ERR("hddtemp gethostbyname_r: %s", hstrerror(h_errno));
-#else /* HAVE_GETHOSTBYNAME_R */
-	if (!(he_res = gethostbyname(hostaddr))) {
-		perror("gethostbyname()");
-#endif /* HAVE_GETHOSTBYNAME_R */
-		goto GET_OUT;
 	}
-
+}
-	addr.sin_family = AF_INET;
+
-	addr.sin_port = htons(port);
+int get_hddtemp_info(const char *dev, short *val, char *unit)
-	addr.sin_addr = *((struct in_addr *) he_res->h_addr);
+{
-	memset(&(addr.sin_zero), 0, 8);
+	struct hdd_info *hdi = hdd_info_head.next;
-
+
-	if (connect(sockfd, (struct sockaddr *) &addr,
+	/* if no dev is given, just use hdd_info_head->next */
-				sizeof(struct sockaddr)) == -1) {
+	while(dev && hdi) {
-		perror("connect");
+		if (!strcmp(dev, hdi->dev))
-		goto GET_OUT;
+			break;
-	}
+		hdi = hdi->next;
-
+	}
-	FD_ZERO(&rfds);
+	if (!hdi)
-	FD_SET(sockfd, &rfds);
+		return 1;
-
+	
-	/* We're going to wait up to a half second to see whether there's any
+	*val = hdi->temp;
-	 * data available. Polling with timeout set to 0 doesn't seem to work
+	*unit = hdi->unit;
-	 * with hddtemp.
+	return 0;
-	 */
-	tv.tv_sec = 0;
-	tv.tv_usec = 500000;
-
-	i = select(sockfd + 1, &rfds, NULL, NULL, &tv);
-	if (i == -1) { /* select() failed */
-		if (errno == EINTR) {
-			/* silently ignore interrupted system call */
-			goto GET_OUT;
-		} else {
-			perror("select");
-		}
-	} else if (i == 0) { /* select() timeouted */
-		NORM_ERR("hddtemp had nothing for us");
-		goto GET_OUT;
-	}
-
-	p = buf;
-	len = 0;
-	do {
-		i = recv(sockfd, p, BUFLEN - (p - buf), 0);
-		if (i < 0) {
-			perror("recv");
-			break;
-		}
-		len += i;
-		p += i;
-	} while (i > 0 && p < buf + BUFLEN - 1);
-
-	if (len < 2) {
-		NORM_ERR("hddtemp returned nada");
-		goto GET_OUT;
-	}
-
-	buf[len] = 0;
-
-	if ((p = read_hdd_val(buf)) == NULL)
-		goto GET_OUT;
-	do {
-		if (!strncmp(dev, p, strlen(dev)))
-			asprintf(&r, "%s", p + strlen(dev));
-		free(p);
-	} while(!r && (p = read_hdd_val(NULL)) != NULL);
-
-GET_OUT:
-	close(sockfd);
-	return r;
 }

src/hddtemp.h

@@ -3,7 +3,10 @@
 #ifndef HDDTEMP_H_
 #define HDDTEMP_H_
 
-int scan_hddtemp(const char *arg, char **dev, char **addr, int *port);
+void set_hddtemp_host(const char *);
-char *get_hddtemp_info(char *dev, char *addr, int port);
+void set_hddtemp_port(const char *);
+void update_hddtemp(void);
+void free_hddtemp(void);
+int get_hddtemp_info(const char *, short *, char *);
 
 #endif /*HDDTEMP_H_*/