- determine maximum buffer length for SDR reading dynamically

- add support for "sensor get" command
- let sensor get and sensor thresh use sensor id as well as name
- build list of sdrs when searching, for use in xref of sel data
- fix memory leak during sensor list

ipmitool/include/ipmitool/ipmi_sdr.h

@@ -98,7 +98,6 @@ struct sdr_get_rq {
 	unsigned short	id;		/* record ID */
 	unsigned char	offset;		/* offset into SDR */
 #define GET_SDR_ENTIRE_RECORD	0xff
-#define GET_SDR_MAX_LEN		30
 	unsigned char	length;		/* length to read */
 } __attribute__ ((packed));
 
@@ -630,7 +629,9 @@ float sdr_convert_sensor_reading(struct sdr_record_full_sensor * sensor, unsigne
 unsigned char sdr_convert_sensor_value_to_raw(struct sdr_record_full_sensor * sensor, float val);
 struct ipmi_rs * ipmi_sdr_get_sensor_reading(struct ipmi_intf * intf, unsigned char sensor);
 const char * ipmi_sdr_get_sensor_type_desc(const unsigned char type);
-struct sdr_record_full_sensor * ipmi_sdr_find_sdr(struct ipmi_intf * intf, char * id);
 
+struct sdr_record_full_sensor *ipmi_sdr_find_sdr_byid(struct ipmi_intf * intf, char * id);
+struct sdr_record_full_sensor *ipmi_sdr_find_sdr_bynum(struct ipmi_intf * intf, unsigned char num);
+void ipmi_sdr_list_empty(void);
 
 #endif  /* IPMI_SDR_H */

ipmitool/include/ipmitool/ipmi_sensor.h

@@ -88,5 +88,7 @@ struct sensor_set_thresh_rq {
 
 
 int ipmi_sensor_main(struct ipmi_intf *, int, char **);
+void ipmi_sensor_print_full(struct ipmi_intf *, struct sdr_record_full_sensor *);
+void ipmi_sensor_print_compact(struct ipmi_intf *, struct sdr_record_compact_sensor *);
 
 #endif  /* IPMI_SENSOR_H */

ipmitool/lib/ipmi_sdr.c

@@ -47,6 +47,13 @@
 #endif
 
 extern int verbose;
+static int sdr_max_read_len = GET_SDR_ENTIRE_RECORD;
+
+struct sdr_records_full {
+	struct sdr_record_full_sensor * record;
+	struct sdr_records_full * next;
+};
+static struct sdr_records_full * sdr_list_head = NULL;
 
 /* convert unsigned value to 2's complement signed */
 int utos(unsigned val, unsigned bits)
@@ -945,12 +952,12 @@ ipmi_sdr_get_record(struct ipmi_intf * intf, struct sdr_get_rs * header,
 	req.msg.data = (unsigned char *)&sdr_rq;
 	req.msg.data_len = sizeof(sdr_rq);
 
-	/* read SDR record with partial (30 byte) reads
+	/* read SDR record with partial reads
-	 * because a full read (0xff) exceeds the maximum
+	 * because a full read usually exceeds the maximum
 	 * transport buffer size.  (completion code 0xca)
 	 */
 	while (i < len) {
-		sdr_rq.length = (len-i < GET_SDR_MAX_LEN) ? len-i : GET_SDR_MAX_LEN;
+		sdr_rq.length = (len-i < sdr_max_read_len) ? len-i : sdr_max_read_len;
 		sdr_rq.offset = i+5; /* 5 header bytes */
 		if (verbose > 1)
 			printf("getting %d bytes from SDR at offset %d\n",
@@ -962,9 +969,16 @@ ipmi_sdr_get_record(struct ipmi_intf * intf, struct sdr_get_rs * header,
 			return NULL;
 		}
 
-		if (rsp->ccode == 0xc5) {
+		switch (rsp->ccode) {
+		case 0xca:
+			/* read too many bytes at once */
+			sdr_max_read_len = (sdr_max_read_len >> 1) - 1;
+			continue;
+		case 0xc5:
+			/* lost reservation */
 			if (verbose > 1)
-				printf("SDR reserveration canceled. Sleeping a bit and retrying...\n");
+				printf("SDR reserveration canceled. "
+				       "Sleeping a bit and retrying...\n");
 
 			sleep (rand () & 3);
 
@@ -977,12 +991,12 @@ ipmi_sdr_get_record(struct ipmi_intf * intf, struct sdr_get_rs * header,
 		}
 
 		if (!rsp->data_len || rsp->ccode) {
-			free (data);
+			free(data);
 			return NULL;
 		}
 
 		memcpy(data+i, rsp->data+2, sdr_rq.length);
-		i+=GET_SDR_MAX_LEN;
+		i += sdr_max_read_len;
 	}
 
 	return data;
@@ -994,31 +1008,102 @@ ipmi_sdr_end(struct ipmi_intf * intf, struct ipmi_sdr_iterator * itr)
 	free (itr);
 }
 
-struct sdr_record_full_sensor *
+int ipmi_sdr_list_fill(struct ipmi_intf * intf)
-ipmi_sdr_find_sdr(struct ipmi_intf * intf, char * id)
 {
 	struct sdr_get_rs * header;
 	struct ipmi_sdr_iterator * itr;
+	static struct sdr_records_full * sdr_list_tail;
 
 	itr = ipmi_sdr_start(intf);
-    if (!itr)
+	if (!itr) {
-    {
 		printf("Unable to open SDR for reading\n");
-        return;
+		return -1;
 	}
 
-    while (header = ipmi_sdr_get_next_header(intf, itr)) 
+	while (header = ipmi_sdr_get_next_header(intf, itr)) {
-    {
 		struct sdr_record_full_sensor * sdr;
+		struct sdr_records_full * sdrr;
+
 		if (header->type != SDR_RECORD_TYPE_FULL_SENSOR)
 			continue;
+
 		sdr = (struct sdr_record_full_sensor *)ipmi_sdr_get_record(intf, header, itr);
-        if (sdr && !strncmp(sdr->id_string, id, sdr->id_code & 0x3f))
+		if (!sdr)
-            return sdr;
+			continue;
+
+		sdrr = malloc(sizeof(struct sdr_records_full));
+		memset(sdrr, 0, sizeof(struct sdr_records_full));
+
+		sdrr->record = sdr;
+
+		if (!sdr_list_head)
+			sdr_list_head = sdrr;
+		else
+			sdr_list_tail->next = sdrr;
+		sdr_list_tail = sdrr;
+
+		if (verbose > 3)
+			printf("added list entry %x\n", sdrr->record->keys.sensor_num);
 	}
+
+	ipmi_sdr_end(intf, itr);
+
+	return 0;
+}
+
+struct sdr_record_full_sensor *
+ipmi_sdr_find_sdr_byid(struct ipmi_intf * intf, char * id)
+{
+	static struct sdr_records_full * e;
+
+	if (!sdr_list_head)
+		ipmi_sdr_list_fill(intf);
+
+	e = sdr_list_head;
+	while (e) {
+		if (!strncmp(e->record->id_string, id, e->record->id_code & 0x3f))
+			return e->record;
+		e = e->next;
+	}
+
 	return NULL;
 }
 
+struct sdr_record_full_sensor *
+ipmi_sdr_find_sdr_bynum(struct ipmi_intf * intf, unsigned char num)
+{
+	static struct sdr_records_full * e;
+
+	if (!sdr_list_head)
+		ipmi_sdr_list_fill(intf);
+
+	e = sdr_list_head;
+
+	while (e) {
+		if (e->record->keys.sensor_num == num)
+			return e->record;
+		e = e->next;
+	}
+
+	return NULL;
+}
+
+void ipmi_sdr_list_empty(void)
+{
+	struct sdr_records_full *list, *next;
+
+	list = sdr_list_head;
+	while (list) {
+		if (verbose > 3)
+			printf("cleared sdr entry %x\n", list->record->keys.sensor_num);
+		if (list->record)
+			free(list->record);
+		next = list->next;
+		free(list);
+		list = next;
+	}
+}
+
 int ipmi_sdr_main(struct ipmi_intf * intf, int argc, char ** argv)
 {
 	srand (time (NULL));

ipmitool/lib/ipmi_sensor.c

@@ -428,8 +428,7 @@ ipmi_sensor_print_full_analog(struct ipmi_intf * intf,
     }
 }
 
-static void
+void ipmi_sensor_print_full(struct ipmi_intf * intf,
-ipmi_sensor_print_full(struct ipmi_intf * intf,
                        struct sdr_record_full_sensor * sensor)
 {
     if (sensor->unit.analog != 3)
@@ -438,8 +437,7 @@ ipmi_sensor_print_full(struct ipmi_intf * intf,
         ipmi_sensor_print_full_discrete(intf, sensor);
 }
 
-static void
+void ipmi_sensor_print_compact(struct ipmi_intf * intf,
-ipmi_sensor_print_compact(struct ipmi_intf * intf,
                           struct sdr_record_compact_sensor * sensor)
 {
     char id[17];
@@ -596,11 +594,10 @@ ipmi_sensor_list(struct ipmi_intf * intf)
             case SDR_RECORD_TYPE_COMPACT_SENSOR:
                 ipmi_sensor_print_compact(intf, (struct sdr_record_compact_sensor *) rec);
                 break;
-            default:
-                continue;
-                break;
         }
+	free(rec);
     }
+    ipmi_sdr_end(intf, itr);
 }
 
 const struct valstr threshold_vals[] = {
@@ -626,7 +623,7 @@ ipmi_sensor_set_threshold(struct ipmi_intf * intf, int argc, char ** argv)
     if (argc < 3 || !strncmp(argv[0], "help", 4))
     {
             printf("sensor thresh <id> <threshold> <setting>\n");
-            printf("   id        : name of the sensor for which threshold is to be set\n");
+            printf("   id        : name or number of the sensor for which threshold is to be set\n");
             printf("   threshold : which threshold to set\n");
             printf("                 unr = upper non-recoverable\n");
             printf("                 ucr = upper critical\n");
@@ -672,11 +669,18 @@ ipmi_sensor_set_threshold(struct ipmi_intf * intf, int argc, char ** argv)
     }
 
     printf("Locating sensor record...\n");
-    sdr = ipmi_sdr_find_sdr(intf, id);
+    /* lookup by sensor name */
+    sdr = ipmi_sdr_find_sdr_byid(intf, id);
+    if (!sdr)
+    {
+	/* lookup by sensor number */
+	unsigned char num = (unsigned char)strtol(id, NULL, 0);
+	sdr = ipmi_sdr_find_sdr_bynum(intf, num);
+    }
     if (sdr)
     {
 	printf("Setting sensor \"%s\" %s threshold to %.3f\n",
-	       id, val2str(settingMask, threshold_vals), setting);
+	       sdr->id_string, val2str(settingMask, threshold_vals), setting);
         rsp = ipmi_sensor_set_sensor_thresholds(intf, 
                                                 sdr->keys.sensor_num,
                                                 settingMask,
@@ -687,8 +691,38 @@ ipmi_sensor_set_threshold(struct ipmi_intf * intf, int argc, char ** argv)
     else
     {
         printf("Sensor data record not found!\n");
+    }
+
+    ipmi_sdr_list_empty();
+}
+
+static void ipmi_sensor_get(struct ipmi_intf * intf, char * id)
+{
+	struct sdr_record_full_sensor * sdr;
+
+	if (!id || !strncmp(id, "help", 4)) {
+		printf("sensor get <id>\n");
+		printf("   id        : name or number desired sensor\n");
 		return;
 	}
+	printf("Locating sensor record...\n");
+
+	/* lookup by sensor name */
+	sdr = ipmi_sdr_find_sdr_byid(intf, id);
+	if (!sdr) {
+		/* lookup by sensor number */
+		unsigned char num = (unsigned char)strtol(id, NULL, 0);
+		sdr = ipmi_sdr_find_sdr_bynum(intf, num);
+	}
+
+	if (sdr) {
+		verbose = verbose ? : 1;
+		ipmi_sensor_print_full(intf, sdr);
+	} else {
+		printf("Sensor data record not found!\n");
+	}
+
+	ipmi_sdr_list_empty();
 }
 
 int
@@ -697,7 +731,7 @@ ipmi_sensor_main(struct ipmi_intf * intf, int argc, char ** argv)
 	if (!argc)
 		ipmi_sensor_list(intf);
 	else if (!strncmp(argv[0], "help", 4)) {
-		printf("Sensor Commands:  list thresh\n");
+		printf("Sensor Commands:  list thresh get\n");
 	}
 	else if (!strncmp(argv[0], "list", 4)) {
 		ipmi_sensor_list(intf);
@@ -705,6 +739,9 @@ ipmi_sensor_main(struct ipmi_intf * intf, int argc, char ** argv)
 	else if (!strncmp(argv[0], "thresh", 5)) {
 		ipmi_sensor_set_threshold(intf, argc-1, &argv[1]);
 	}
+	else if (!strncmp(argv[0], "get", 3)) {
+		ipmi_sensor_get(intf, argv[1]);
+	}
 	else
 		printf("Invalid sensor command: %s\n", argv[0]);
 	return 0;