New upstream version 1.8.19upstream/1.8.19upstream

1 files changed, 474 insertions, 475 deletions

diff --git a/lib/ipmi_fru.c b/lib/ipmi_fru.c
index cf00eff..3d1d8a1 100644
--- a/lib/ipmi_fru.c
+++ b/lib/ipmi_fru.c
@@ -33,12 +33,15 @@
 #include <ipmitool/ipmi.h>
 #include <ipmitool/log.h>
 #include <ipmitool/helper.h>
+#include <ipmitool/ipmi_cc.h>
 #include <ipmitool/ipmi_intf.h>
 #include <ipmitool/ipmi_fru.h>
 #include <ipmitool/ipmi_mc.h>
 #include <ipmitool/ipmi_sdr.h>
 #include <ipmitool/ipmi_strings.h>  /* IANA id strings */
+#include <ipmitool/ipmi_time.h>
 
+#include <stdbool.h>
 #include <stdlib.h>
 #include <string.h>
 #include <time.h>
@@ -49,6 +52,7 @@
 #endif
 
 #define FRU_MULTIREC_CHUNK_SIZE     (255 + sizeof(struct fru_multirec_header))
+#define FRU_FIELD_VALID(a) (a && a[0])
 
 static const char *section_id[4] = {
 	"Internal Use Section",
@@ -57,16 +61,56 @@ static const char *section_id[4] = {
 	"Product Section"
 };
 
+static const char * combined_voltage_desc[] = {
+	"12 V",
+	"-12 V",
+	"5 V",
+	"3.3 V"
+};
+
+static const char * chassis_type_desc[] = {
+	"Unspecified",
+	"Other",
+	"Unknown",
+	"Desktop",
+	"Low Profile Desktop",
+	"Pizza Box",
+	"Mini Tower",
+	"Tower",
+	"Portable",
+	"LapTop",
+	"Notebook",
+	"Hand Held",
+	"Docking Station",
+	"All in One",
+	"Sub Notebook",
+	"Space-saving",
+	"Lunch Box",
+	"Main Server Chassis",
+	"Expansion Chassis",
+	"SubChassis",
+	"Bus Expansion Chassis",
+	"Peripheral Chassis",
+	"RAID Chassis",
+	"Rack Mount Chassis",
+	"Sealed-case PC",
+	"Multi-system Chassis",
+	"CompactPCI",
+	"AdvancedTCA",
+	"Blade",
+	"Blade Enclosure"
+};
+
+static inline bool fru_cc_rq2big(int code) {
+	return (code == IPMI_CC_REQ_DATA_INV_LENGTH
+		|| code == IPMI_CC_REQ_DATA_FIELD_EXCEED
+		|| code == IPMI_CC_CANT_RET_NUM_REQ_BYTES);
+}
+
 /* From lib/dimm_spd.c: */
 int
 ipmi_spd_print_fru(struct ipmi_intf * intf, uint8_t id);
 
-/* From src/plugins/ipmi_intf.c: */
-void
-ipmi_intf_set_max_request_data_size(struct ipmi_intf * intf, uint16_t size);
-void
-ipmi_intf_set_max_response_data_size(struct ipmi_intf * intf, uint16_t size);
-
 extern int verbose;
 
 static void ipmi_fru_read_to_bin(struct ipmi_intf * intf, char * pFileName, uint8_t fruId);
@@ -107,7 +151,7 @@ char * get_fru_area_str(uint8_t * data, uint32_t * offset)
 {
 	static const char bcd_plus[] = "0123456789 -.:,_";
 	char * str;
-	int len, off, size, i, j, k, typecode;
+	int len, off, size, i, j, k, typecode, char_idx;
 	union {
 		uint32_t bits;
 		char chars[4];
@@ -126,15 +170,15 @@ char * get_fru_area_str(uint8_t * data, uint32_t * offset)
 
 	switch (typecode) {
 	case 0:           /* 00b: binary/unspecified */
-		/* hex dump -> 2x length */
-		size = (len*2);
+	case 1:           /* 01b: BCD plus */
+		/* hex dump or BCD -> 2x length */
+		size = (len * 2);
 		break;
 	case 2:           /* 10b: 6-bit ASCII */
-		/* 4 chars per group of 1-3 bytes */
-		size = ((((len+2)*4)/3) & ~3);
+		/* 4 chars per group of 1-3 bytes, round up to 4 bytes boundary */
+		size = (len / 3 + 1) * 4;
 		break;
 	case 3:           /* 11b: 8-bit ASCII */
-	case 1:           /* 01b: BCD plus */
 		/* no length adjustment */
 		size = len;
 		break;
@@ -145,11 +189,11 @@ char * get_fru_area_str(uint8_t * data, uint32_t * offset)
 		return NULL;
 	}
 	str = malloc(size+1);
-	if (str == NULL)
+	if (!str)
 		return NULL;
 	memset(str, 0, size+1);
 
-	if (len == 0) {
+	if (size == 0) {
 		str[0] = '\0';
 		*offset = off;
 		return str;
@@ -157,30 +201,30 @@ char * get_fru_area_str(uint8_t * data, uint32_t * offset)
 
 	switch (typecode) {
 	case 0:        /* Binary */
-		strncpy(str, buf2str(&data[off], len), len*2);
+		strncpy(str, buf2str(&data[off], len), size);
 		break;
 
 	case 1:        /* BCD plus */
-		for (k=0; k<len; k++)
-			str[k] = bcd_plus[(data[off+k] & 0x0f)];
+		for (k = 0; k < size; k++)
+			str[k] = bcd_plus[((data[off + k / 2] >> ((k % 2) ? 0 : 4)) & 0x0f)];
 		str[k] = '\0';
 		break;
 
 	case 2:        /* 6-bit ASCII */
-		for (i=j=0; i<len; i+=3) {
+		for (i = j = 0; i < len; i += 3) {
 			u.bits = 0;
-			k = ((len-i) < 3 ? (len-i) : 3);
+			k = ((len - i) < 3 ? (len - i) : 3);
 #if WORDS_BIGENDIAN
 			u.chars[3] = data[off+i];
 			u.chars[2] = (k > 1 ? data[off+i+1] : 0);
 			u.chars[1] = (k > 2 ? data[off+i+2] : 0);
-#define CHAR_IDX 3
+			char_idx = 3;
 #else
 			memcpy((void *)&u.bits, &data[off+i], k);
-#define CHAR_IDX 0
+			char_idx = 0;
 #endif
 			for (k=0; k<4; k++) {
-				str[j++] = ((u.chars[CHAR_IDX] & 0x3f) + 0x20);
+				str[j++] = ((u.chars[char_idx] & 0x3f) + 0x20);
 				u.bits >>= 6;
 			}
 		}
@@ -188,8 +232,8 @@ char * get_fru_area_str(uint8_t * data, uint32_t * offset)
 		break;
 
 	case 3:
-		memcpy(str, &data[off], len);
-		str[len] = '\0';
+		memcpy(str, &data[off], size);
+		str[size] = '\0';
 		break;
 	}
 
@@ -204,26 +248,26 @@ char * get_fru_area_str(uint8_t * data, uint32_t * offset)
  * input_filename - user input string
  *
  * returns   0  if path is ok
- * returns (-1) if path is NULL
- * returns (-2) if path is too short
- * returns (-3) if path is too long
+ * returns -1 if path is NULL
+ * returns -2 if path is too short
+ * returns -3 if path is too long
  */
 int
 is_valid_filename(const char *input_filename)
 {
-	if (input_filename == NULL) {
+	if (!input_filename) {
 		lprintf(LOG_ERR, "ERROR: NULL pointer passed.");
-		return (-1);
+		return -1;
 	}
 
 	if (strlen(input_filename) < 1) {
 		lprintf(LOG_ERR, "File/path is invalid.");
-		return (-2);
+		return -2;
 	}
 
 	if (strlen(input_filename) >= 512) {
 		lprintf(LOG_ERR, "File/path must be shorter than 512 bytes.");
-		return (-3);
+		return -3;
 	}
 
 	return 0;
@@ -271,12 +315,12 @@ build_fru_bloc(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id)
 
 	rsp = intf->sendrecv(intf, &req);
 
-	if (rsp == NULL) {
+	if (!rsp) {
 		lprintf(LOG_ERR, " Device not present (No Response)");
 		return NULL;
 	}
 
-	if (rsp->ccode > 0) {
+	if (rsp->ccode) {
 		lprintf(LOG_ERR," Device not present (%s)",
 				val2str(rsp->ccode, completion_code_vals));
 		return NULL;
@@ -428,11 +472,15 @@ free_fru_bloc(t_ipmi_fru_bloc *bloc)
 	while (bloc) {
 		del = bloc;
 		bloc = bloc->next;
-		free(del);
-		del = NULL;
+		free_n(&del);
 	}
 }
 
+/* By how many bytes to reduce a write command on a size failure. */
+#define FRU_BLOCK_SZ	8
+/* Baseline for a large enough piece to reduce via steps instead of bytes. */
+#define FRU_AREA_MAXIMUM_BLOCK_SZ	32
+
 /*
  * write FRU[doffset:length] from the pFrubuf[soffset:length]
  * rc=1 on success
@@ -458,7 +506,7 @@ write_fru_area(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id,
 
 	if (fru->access && ((doffset & 1) || (length & 1))) {
 		lprintf(LOG_ERROR, "Odd offset or length specified");
-		return (-1);
+		return -1;
 	}
 
 	t_ipmi_fru_bloc * fru_bloc = build_fru_bloc(intf, fru, id);
@@ -554,20 +602,20 @@ write_fru_area(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id,
 			break;
 		}
 
-		if (rsp->ccode == 0xc7 || rsp->ccode == 0xc8 || rsp->ccode == 0xca) {
-			if (fru->max_write_size > 8) {
-				fru->max_write_size -= 8;
+		if (fru_cc_rq2big(rsp->ccode)) {
+			if (fru->max_write_size > FRU_AREA_MAXIMUM_BLOCK_SZ) {
+				fru->max_write_size -= FRU_BLOCK_SZ;
 				lprintf(LOG_INFO, "Retrying FRU write with request size %d",
 						fru->max_write_size);
 				continue;
 			}
-		} else if(rsp->ccode == 0x80) {
-			rsp->ccode = 0;
+		} else if (rsp->ccode == IPMI_CC_FRU_WRITE_PROTECTED_OFFSET) {
+			rsp->ccode = IPMI_CC_OK;
 			// Write protected section
 			protected_bloc = 1;
 		}
 
-		if (rsp->ccode > 0)
+		if (rsp->ccode)
 			break;
 
 		if (protected_bloc == 0) {
@@ -615,7 +663,10 @@ int
 read_fru_area(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id,
 			uint32_t offset, uint32_t length, uint8_t *frubuf)
 {
-	uint32_t off = offset, tmp, finish;
+	uint32_t off = offset;
+	uint32_t tmp;
+	uint32_t finish;
+	uint32_t size_left_in_buffer;
 	struct ipmi_rs * rsp;
 	struct ipmi_rq req;
 	uint8_t msg_data[4];
@@ -628,10 +679,12 @@ read_fru_area(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id,
 
 	finish = offset + length;
 	if (finish > fru->size) {
+		memset(frubuf + fru->size, 0, length - fru->size);
 		finish = fru->size;
 		lprintf(LOG_NOTICE, "Read FRU Area length %d too large, "
 			"Adjusting to %d",
 			offset + length, finish - offset);
+		length = finish - offset;
 	}
 
 	memset(&req, 0, sizeof(req));
@@ -667,6 +720,7 @@ read_fru_area(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id,
 		}
 	}
 
+	size_left_in_buffer = length;
 	do {
 		tmp = fru->access ? off >> 1 : off;
 		msg_data[0] = id;
@@ -679,18 +733,19 @@ read_fru_area(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id,
 			msg_data[3] = (uint8_t)tmp;
 
 		rsp = intf->sendrecv(intf, &req);
-		if (rsp == NULL) {
+		if (!rsp) {
 			lprintf(LOG_NOTICE, "FRU Read failed");
 			break;
 		}
-		if (rsp->ccode > 0) {
+		if (rsp->ccode) {
 			/* if we get C7h or C8h or CAh return code then we requested too
 			* many bytes at once so try again with smaller size */
-			if ((rsp->ccode == 0xc7 || rsp->ccode == 0xc8 || rsp->ccode == 0xca)
-					&& fru->max_read_size > 8) {
-				if (fru->max_read_size > 32) {
+			if (fru_cc_rq2big(rsp->ccode)
+			    && fru->max_read_size > FRU_BLOCK_SZ)
+			{
+				if (fru->max_read_size > FRU_AREA_MAXIMUM_BLOCK_SZ) {
 					/* subtract read length more aggressively */
-					fru->max_read_size -= 8;
+					fru->max_read_size -= FRU_BLOCK_SZ;
 				} else {
 					/* subtract length less aggressively */
 					fru->max_read_size--;
@@ -707,9 +762,18 @@ read_fru_area(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id,
 		}
 
 		tmp = fru->access ? rsp->data[0] << 1 : rsp->data[0];
+		if(rsp->data_len < 1
+		   || tmp > rsp->data_len - 1
+		   || tmp > size_left_in_buffer)
+		{
+			printf(" Not enough buffer size");
+			return -1;
+		}
+
 		memcpy(frubuf, rsp->data + 1, tmp);
 		off += tmp;
 		frubuf += tmp;
+		size_left_in_buffer -= tmp;
 		/* sometimes the size returned in the Info command
 		* is too large.  return 0 so higher level function
 		* still attempts to parse what was returned */
@@ -742,7 +806,9 @@ read_fru_area_section(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id,
 			uint32_t offset, uint32_t length, uint8_t *frubuf)
 {
 	static uint32_t fru_data_rqst_size = 20;
-	uint32_t off = offset, tmp, finish;
+	uint32_t off = offset;
+	uint32_t tmp, finish;
+	uint32_t size_left_in_buffer;
 	struct ipmi_rs * rsp;
 	struct ipmi_rq req;
 	uint8_t msg_data[4];
@@ -755,10 +821,12 @@ read_fru_area_section(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id,
 
 	finish = offset + length;
 	if (finish > fru->size) {
+		memset(frubuf + fru->size, 0, length - fru->size);
 		finish = fru->size;
 		lprintf(LOG_NOTICE, "Read FRU Area length %d too large, "
 			"Adjusting to %d",
 			offset + length, finish - offset);
+		length = finish - offset;
 	}
 
 	memset(&req, 0, sizeof(req));
@@ -773,6 +841,8 @@ read_fru_area_section(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id,
 	if (fru->access && fru_data_rqst_size > 16)
 #endif
 		fru_data_rqst_size = 16;
+
+	size_left_in_buffer = length;
 	do {
 		tmp = fru->access ? off >> 1 : off;
 		msg_data[0] = id;
@@ -785,16 +855,16 @@ read_fru_area_section(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id,
 			msg_data[3] = (uint8_t)tmp;
 
 		rsp = intf->sendrecv(intf, &req);
-		if (rsp == NULL) {
+		if (!rsp) {
 			lprintf(LOG_NOTICE, "FRU Read failed");
 			break;
 		}
-		if (rsp->ccode > 0) {
+		if (rsp->ccode) {
 			/* if we get C7 or C8  or CA return code then we requested too
 			* many bytes at once so try again with smaller size */
-			if ((rsp->ccode == 0xc7 || rsp->ccode == 0xc8 || rsp->ccode == 0xca) &&
-				(--fru_data_rqst_size > 8)) {
-				lprintf(LOG_INFO, "Retrying FRU read with request size %d",
+			if (fru_cc_rq2big(rsp->ccode) && (--fru_data_rqst_size > FRU_BLOCK_SZ)) {
+				lprintf(LOG_INFO,
+					"Retrying FRU read with request size %d",
 					fru_data_rqst_size);
 				continue;
 			}
@@ -804,8 +874,16 @@ read_fru_area_section(struct ipmi_intf * intf, struct fru_info *fru, uint8_t id,
 		}
 
 		tmp = fru->access ? rsp->data[0] << 1 : rsp->data[0];
+		if(rsp->data_len < 1
+		   || tmp > rsp->data_len - 1
+		   || tmp > size_left_in_buffer)
+		{
+			printf(" Not enough buffer size");
+			return -1;
+		}
 		memcpy((frubuf + off)-offset, rsp->data + 1, tmp);
 		off += tmp;
+		size_left_in_buffer -= tmp;
 
 		/* sometimes the size returned in the Info command
 		* is too large.  return 0 so higher level function
@@ -834,7 +912,7 @@ fru_area_print_multirec_bloc(struct ipmi_intf * intf, struct fru_info * fru,
 	i = last_off = offset;
 
 	fru_data = malloc(fru->size + 1);
-	if (fru_data == NULL) {
+	if (!fru_data) {
 		lprintf(LOG_ERR, " Out of memory!");
 		return;
 	}
@@ -878,8 +956,7 @@ fru_area_print_multirec_bloc(struct ipmi_intf * intf, struct fru_info * fru,
 
 	lprintf(LOG_DEBUG ,"Multi-Record area ends at: %i (%xh)",i,i);
 
-	free(fru_data);
-	fru_data = NULL;
+	free_n(&fru_data);
 }
 
 
@@ -898,6 +975,7 @@ fru_area_print_chassis(struct ipmi_intf * intf, struct fru_info * fru,
 	uint8_t * fru_data;
 	uint32_t fru_len, i;
 	uint8_t tmp[2];
+	size_t chassis_type;
 
 	fru_len = 0;
 
@@ -911,7 +989,7 @@ fru_area_print_chassis(struct ipmi_intf * intf, struct fru_info * fru,
 	}
 
 	fru_data = malloc(fru_len);
-	if (fru_data == NULL) {
+	if (!fru_data) {
 		lprintf(LOG_ERR, "ipmitool: malloc failure");
 		return;
 	}
@@ -920,8 +998,7 @@ fru_area_print_chassis(struct ipmi_intf * intf, struct fru_info * fru,
 
 	/* read in the full fru */
 	if (read_fru_area(intf, fru, id, offset, fru_len, fru_data) < 0) {
-		free(fru_data);
-		fru_data = NULL;
+		free_n(&fru_data);
 		return;
 	}
 
@@ -931,42 +1008,38 @@ fru_area_print_chassis(struct ipmi_intf * intf, struct fru_info * fru,
 	 */
 	i = 2;
 
-	printf(" Chassis Type          : %s\n",
- 		chassis_type_desc[fru_data[i] >
- 		(sizeof(chassis_type_desc)/sizeof(chassis_type_desc[0])) - 1 ?
- 		2 : fru_data[i]]);
+	chassis_type = (fru_data[i] > ARRAY_SIZE(chassis_type_desc) - 1)
+	               ? 2
+	               : fru_data[i];
+	printf(" Chassis Type          : %s\n", chassis_type_desc[chassis_type]);
 
  	i++;
 
 	fru_area = get_fru_area_str(fru_data, &i);
-	if (fru_area != NULL) {
+	if (fru_area) {
 		if (strlen(fru_area) > 0) {
 			printf(" Chassis Part Number   : %s\n", fru_area);
 		}
-		free(fru_area);
-		fru_area = NULL;
+		free_n(&fru_area);
 	}
 
 	fru_area = get_fru_area_str(fru_data, &i);
-	if (fru_area != NULL) {
+	if (fru_area) {
 		if (strlen(fru_area) > 0) {
 			printf(" Chassis Serial        : %s\n", fru_area);
 		}
-		free(fru_area);
-		fru_area = NULL;
+		free_n(&fru_area);
 	}
 
 	/* read any extra fields */
-	while ((fru_data[i] != 0xc1) && (i < fru_len))
-	{
+	while ((i < fru_len) && (fru_data[i] != FRU_END_OF_FIELDS)) {
 		int j = i;
 		fru_area = get_fru_area_str(fru_data, &i);
-		if (fru_area != NULL) {
+		if (fru_area) {
 			if (strlen(fru_area) > 0) {
 				printf(" Chassis Extra         : %s\n", fru_area);
 			}
-			free(fru_area);
-			fru_area = NULL;
+			free_n(&fru_area);
 		}
 
 		if (i == j) {
@@ -974,10 +1047,7 @@ fru_area_print_chassis(struct ipmi_intf * intf, struct fru_info * fru,
 		}
 	}
 
-	if (fru_data != NULL) {
-		free(fru_data);
-		fru_data = NULL;
-	}
+	free_n(&fru_data);
 }
 
 /* fru_area_print_board  -  Print FRU Board Area
@@ -995,7 +1065,7 @@ fru_area_print_board(struct ipmi_intf * intf, struct fru_info * fru,
 	uint8_t * fru_data;
 	uint32_t fru_len;
 	uint32_t i;
-	time_t tval;
+	time_t ts;
 	uint8_t tmp[2];
 
 	fru_len = 0;
@@ -1010,7 +1080,7 @@ fru_area_print_board(struct ipmi_intf * intf, struct fru_info * fru,
 	}
 
 	fru_data = malloc(fru_len);
-	if (fru_data == NULL) {
+	if (!fru_data) {
 		lprintf(LOG_ERR, "ipmitool: malloc failure");
 		return;
 	}
@@ -1019,8 +1089,7 @@ fru_area_print_board(struct ipmi_intf * intf, struct fru_info * fru,
 
 	/* read in the full fru */
 	if (read_fru_area(intf, fru, id, offset, fru_len, fru_data) < 0) {
-		free(fru_data);
-		fru_data = NULL;
+		free_n(&fru_data);
 		return;
 	}
 
@@ -1031,77 +1100,65 @@ fru_area_print_board(struct ipmi_intf * intf, struct fru_info * fru,
 	 */
 	i = 3;
 
-	tval=((fru_data[i+2] << 16) + (fru_data[i+1] << 8) + (fru_data[i]));
-	tval=tval * 60;
-	tval=tval + secs_from_1970_1996;
-	printf(" Board Mfg Date        : %s", asctime(localtime(&tval)));
+	ts = ipmi_fru2time_t(&fru_data[i]);
+	printf(" Board Mfg Date        : %s\n", ipmi_timestamp_string(ts));
 	i += 3;  /* skip mfg. date time */
 
 	fru_area = get_fru_area_str(fru_data, &i);
-	if (fru_area != NULL) {
+	if (fru_area) {
 		if (strlen(fru_area) > 0) {
 			printf(" Board Mfg             : %s\n", fru_area);
 		}
-		free(fru_area);
-		fru_area = NULL;
+		free_n(&fru_area);
 	}
 
 	fru_area = get_fru_area_str(fru_data, &i);
-	if (fru_area != NULL) {
+	if (fru_area) {
 		if (strlen(fru_area) > 0) {
 			printf(" Board Product         : %s\n", fru_area);
 		}
-		free(fru_area);
-		fru_area = NULL;
+		free_n(&fru_area);
 	}
 
 	fru_area = get_fru_area_str(fru_data, &i);
-	if (fru_area != NULL) {
+	if (fru_area) {
 		if (strlen(fru_area) > 0) {
 			printf(" Board Serial          : %s\n", fru_area);
 		}
-		free(fru_area);
-		fru_area = NULL;
+		free_n(&fru_area);
 	}
 
 	fru_area = get_fru_area_str(fru_data, &i);
-	if (fru_area != NULL) {
+	if (fru_area) {
 		if (strlen(fru_area) > 0) {
 			printf(" Board Part Number     : %s\n", fru_area);
 		}
-		free(fru_area);
-		fru_area = NULL;
+		free_n(&fru_area);
 	}
 
 	fru_area = get_fru_area_str(fru_data, &i);
-	if (fru_area != NULL) {
+	if (fru_area) {
 		if (strlen(fru_area) > 0 && verbose > 0) {
 			printf(" Board FRU ID          : %s\n", fru_area);
 		}
-		free(fru_area);
-		fru_area = NULL;
+		free_n(&fru_area);
 	}
 
 	/* read any extra fields */
-	while ((fru_data[i] != 0xc1) && (i < fru_len))
-	{
+	while ((i < fru_len) && (fru_data[i] != FRU_END_OF_FIELDS)) {
 		int j = i;
 		fru_area = get_fru_area_str(fru_data, &i);
-		if (fru_area != NULL) {
+		if (fru_area) {
 			if (strlen(fru_area) > 0) {
 				printf(" Board Extra           : %s\n", fru_area);
 			}
-			free(fru_area);
-			fru_area = NULL;
+			free_n(&fru_area);
 		}
 		if (i == j)
 			break;
 	}
 
-	if (fru_data != NULL) {
-		free(fru_data);
-		fru_data = NULL;
-	}
+	free_n(&fru_data);
 }
 
 /* fru_area_print_product  -  Print FRU Product Area
@@ -1132,7 +1189,7 @@ fru_area_print_product(struct ipmi_intf * intf, struct fru_info * fru,
 	}
 
 	fru_data = malloc(fru_len);
-	if (fru_data == NULL) {
+	if (!fru_data) {
 		lprintf(LOG_ERR, "ipmitool: malloc failure");
 		return;
 	}
@@ -1142,8 +1199,7 @@ fru_area_print_product(struct ipmi_intf * intf, struct fru_info * fru,
 
 	/* read in the full fru */
 	if (read_fru_area(intf, fru, id, offset, fru_len, fru_data) < 0) {
-		free(fru_data);
-		fru_data = NULL;
+		free_n(&fru_data);
 		return;
 	}
 
@@ -1155,88 +1211,76 @@ fru_area_print_product(struct ipmi_intf * intf, struct fru_info * fru,
 	i = 3;
 
 	fru_area = get_fru_area_str(fru_data, &i);
-	if (fru_area != NULL) {
+	if (fru_area) {
 		if (strlen(fru_area) > 0) {
 			printf(" Product Manufacturer  : %s\n", fru_area);
 		}
-		free(fru_area);
-		fru_area = NULL;
+		free_n(&fru_area);
 	}
 
 	fru_area = get_fru_area_str(fru_data, &i);
-	if (fru_area != NULL) {
+	if (fru_area) {
 		if (strlen(fru_area) > 0) {
 			printf(" Product Name          : %s\n", fru_area);
 		}
-		free(fru_area);
-		fru_area = NULL;
+		free_n(&fru_area);
 	}
 
 	fru_area = get_fru_area_str(fru_data, &i);
-	if (fru_area != NULL) {
+	if (fru_area) {
 		if (strlen(fru_area) > 0) {
 			printf(" Product Part Number   : %s\n", fru_area);
 		}
-		free(fru_area);
-		fru_area = NULL;
+		free_n(&fru_area);
 	}
 
 	fru_area = get_fru_area_str(fru_data, &i);
-	if (fru_area != NULL) {
+	if (fru_area) {
 		if (strlen(fru_area) > 0) {
 			printf(" Product Version       : %s\n", fru_area);
 		}
-		free(fru_area);
-		fru_area = NULL;
+		free_n(&fru_area);
 	}
 
 	fru_area = get_fru_area_str(fru_data, &i);
-	if (fru_area != NULL) {
+	if (fru_area) {
 		if (strlen(fru_area) > 0) {
 			printf(" Product Serial        : %s\n", fru_area);
 		}
-		free(fru_area);
-		fru_area = NULL;
+		free_n(&fru_area);
 	}
 
 	fru_area = get_fru_area_str(fru_data, &i);
-	if (fru_area != NULL) {
+	if (fru_area) {
 		if (strlen(fru_area) > 0) {
 			printf(" Product Asset Tag     : %s\n", fru_area);
 		}
-		free(fru_area);
-		fru_area = NULL;
+		free_n(&fru_area);
 	}
 
 	fru_area = get_fru_area_str(fru_data, &i);
-	if (fru_area != NULL) {
+	if (fru_area) {
 		if (strlen(fru_area) > 0 && verbose > 0) {
 			printf(" Product FRU ID        : %s\n", fru_area);
 		}
-		free(fru_area);
-		fru_area = NULL;
+		free_n(&fru_area);
 	}
 
 	/* read any extra fields */
-	while ((fru_data[i] != 0xc1) && (i < fru_len))
-	{
+	while ((i < fru_len) && (fru_data[i] != FRU_END_OF_FIELDS)) {
 		int j = i;
 		fru_area = get_fru_area_str(fru_data, &i);
-		if (fru_area != NULL) {
+		if (fru_area) {
 			if (strlen(fru_area) > 0) {
 				printf(" Product Extra         : %s\n", fru_area);
 			}
-			free(fru_area);
-			fru_area = NULL;
+			free_n(&fru_area);
 		}
 		if (i == j)
 			break;
 	}
 
-	if (fru_data != NULL) {
-		free(fru_data);
-		fru_data = NULL;
-	}
+	free_n(&fru_data);
 }
 
 /* fru_area_print_multirec  -  Print FRU Multi Record Area
@@ -1262,7 +1306,7 @@ fru_area_print_multirec(struct ipmi_intf * intf, struct fru_info * fru,
 	last_off = offset;
 
 	fru_data = malloc(FRU_MULTIREC_CHUNK_SIZE);
-	if (fru_data == NULL) {
+	if (!fru_data) {
 		lprintf(LOG_ERR, "ipmitool: malloc failure");
 		return;
 	}
@@ -1428,7 +1472,7 @@ fru_area_print_multirec(struct ipmi_intf * intf, struct fru_info * fru,
 
 	lprintf(LOG_DEBUG ,"Multi-Record area ends at: %i (%xh)", last_off, last_off);
 
-	free(fru_data);
+	free_n(&fru_data);
 }
 
 /* ipmi_fru_query_new_value  -  Query new values to replace original FRU content
@@ -1440,16 +1484,18 @@ fru_area_print_multirec(struct ipmi_intf * intf, struct fru_info * fru,
 * returns : TRUE if data changed
 * returns : FALSE if data not changed
 */
-int ipmi_fru_query_new_value(uint8_t *data,int offset, size_t len)
+static
+bool
+ipmi_fru_query_new_value(uint8_t *data,int offset, size_t len)
 {
-	int status=FALSE;
+	bool status = false;
 	int ret;
 	char answer;
 
 	printf("Would you like to change this value <y/n> ? ");
 	ret = scanf("%c", &answer);
 	if (ret != 1) {
-		return FALSE;
+		return false;
 	}
 
 	if( answer == 'y' || answer == 'Y' ){
@@ -1465,17 +1511,16 @@ int ipmi_fru_query_new_value(uint8_t *data,int offset, size_t len)
 		for( i=0;i<len;i++ ){
 			ret = scanf("%x", holder+i);
 			if (ret != 1) {
-				free(holder);
-				return FALSE;
+				free_n(&holder);
+				return false;
 			}
 		}
 		for( i=0;i<len;i++ ){
 			data[offset++] = (unsigned char) *(holder+i);
 		}
 		/* &data[offset++] */
-		free(holder);
-		holder = NULL;
-		status = TRUE;
+		free_n(&holder);
+		status = true;
 	}
 	else{
 		printf("Entered %c\n",answer);
@@ -1572,12 +1617,13 @@ typedef struct OemKontronInformationRecordV1{
 
 */
 
-static void ipmi_fru_oemkontron_get( int argc, char ** argv,uint8_t * fru_data,
-												int off,int len,
-												struct fru_multirec_header *h,
-												struct fru_multirec_oem_header *oh)
+static void ipmi_fru_oemkontron_get(int argc,
+				    char ** argv,
+				    uint8_t * fru_data,
+				    int off,
+				    struct fru_multirec_oem_header *oh)
 {
-	static int badParams=FALSE;
+	static bool badParams = false;
 	int start = off;
 	int offset = start;
 	offset += sizeof(struct fru_multirec_oem_header);
@@ -1585,117 +1631,114 @@ static void ipmi_fru_oemkontron_get( int argc, char ** argv,uint8_t * fru_data,
 	if(!badParams){
 		/* the 'OEM' field is already checked in caller */
 		if( argc > OEM_KONTRON_SUBCOMMAND_ARG_POS ){
-			if(strncmp("oem", argv[OEM_KONTRON_SUBCOMMAND_ARG_POS],3)){
+			if(strcmp("oem", argv[OEM_KONTRON_SUBCOMMAND_ARG_POS])){
 				printf("usage: fru get <id> <oem>\n");
-				badParams = TRUE;
+				badParams = true;
 				return;
 			}
 		}
 		if( argc<GET_OEM_KONTRON_COMPLETE_ARG_COUNT ){
 			printf("usage: oem <iana> <recordid>\n");
 			printf("usage: oem 15000 3\n");
-			badParams = TRUE;
+			badParams = true;
 			return;
 		}
 	}
 
-	if(!badParams){
-
-		if(oh->record_id == OEM_KONTRON_INFORMATION_RECORD ) {
-
-			uint8_t version;
-
-			printf("Kontron OEM Information Record\n");
-			version = oh->record_version;
-
-			uint8_t blockCount;
-			uint8_t blockIndex=0;
+	if (badParams) {
+		return;
+	}
 
-			unsigned int matchInstance = 0;
-			uint8_t instance = 0;
-			
-			if (str2uchar(argv[OEM_KONTRON_INSTANCE_ARG_POS], &instance) != 0) {
-				lprintf(LOG_ERR,
-						"Instance argument '%s' is either invalid or out of range.",
-						argv[OEM_KONTRON_INSTANCE_ARG_POS]);
-				badParams = TRUE;
-				return;
-			}
+	if (oh->record_id != OEM_KONTRON_INFORMATION_RECORD) {
+		return;
+	}
 
-			blockCount = fru_data[offset++];
+	uint8_t version;
 
-			for(blockIndex=0;blockIndex<blockCount;blockIndex++){
-				void * pRecordData;
-				uint8_t nameLen;
+	printf("Kontron OEM Information Record\n");
+	version = oh->record_version;
 
-				nameLen = ( fru_data[offset++] &= 0x3F );
-				printf("  Name: %*.*s\n",nameLen, nameLen, (const char *)(fru_data+offset));
+	uint8_t blockCount;
+	uint8_t blockIndex = 0;
 
-				offset+=nameLen;
+	uint8_t instance = 0;
 
-				pRecordData = &fru_data[offset];
+	if (str2uchar(argv[OEM_KONTRON_INSTANCE_ARG_POS], &instance) != 0) {
+		lprintf(LOG_ERR,
+			"Instance argument '%s' is either invalid or out of range.",
+			argv[OEM_KONTRON_INSTANCE_ARG_POS]);
+		badParams = true;
+		return;
+	}
 
-				printf("  Record Version: %d\n", version);
-				if( version == 0 )
-				{
-					printf("  Version: %*.*s\n",
-						OEM_KONTRON_FIELD_SIZE,
-						OEM_KONTRON_FIELD_SIZE,
-						((tOemKontronInformationRecordV0 *) pRecordData)->field1);
-					printf("  Build Date: %*.*s\n",
-						OEM_KONTRON_FIELD_SIZE,
-						OEM_KONTRON_FIELD_SIZE,
-						((tOemKontronInformationRecordV0 *) pRecordData)->field2);
-					printf("  Update Date: %*.*s\n",
-						OEM_KONTRON_FIELD_SIZE,
-						OEM_KONTRON_FIELD_SIZE,
-						((tOemKontronInformationRecordV0 *) pRecordData)->field3);
-					printf("  Checksum: %*.*s\n\n",
-						OEM_KONTRON_FIELD_SIZE,
-						OEM_KONTRON_FIELD_SIZE,
-						((tOemKontronInformationRecordV0 *) pRecordData)->crc32);
-					matchInstance++;
-					offset+= sizeof(tOemKontronInformationRecordV0);
-					offset++;
-				}
-				else if ( version == 1 )
-				{
-					printf("  Version: %*.*s\n",
-						OEM_KONTRON_VERSION_FIELD_SIZE,
-						OEM_KONTRON_VERSION_FIELD_SIZE,
-						((tOemKontronInformationRecordV1 *) pRecordData)->field1);
-					printf("  Build Date: %*.*s\n",
-						OEM_KONTRON_FIELD_SIZE,
-						OEM_KONTRON_FIELD_SIZE,
-						((tOemKontronInformationRecordV1 *) pRecordData)->field2);
-					printf("  Update Date: %*.*s\n",
-						OEM_KONTRON_FIELD_SIZE,
-						OEM_KONTRON_FIELD_SIZE,
-						((tOemKontronInformationRecordV1 *) pRecordData)->field3);
-					printf("  Checksum: %*.*s\n\n",
-						OEM_KONTRON_FIELD_SIZE,
-						OEM_KONTRON_FIELD_SIZE,
-						((tOemKontronInformationRecordV1 *) pRecordData)->crc32);
-					matchInstance++;
-					offset+= sizeof(tOemKontronInformationRecordV1);
-					offset++;
-				}
-				else
-				{
-					printf ("  Unsupported version %d\n",version);
-				}
-			}
+	blockCount = fru_data[offset++];
+
+	for (blockIndex = 0; blockIndex < blockCount; blockIndex++) {
+		void *pRecordData;
+		uint8_t nameLen;
+
+		nameLen = (fru_data[offset++] &= 0x3F);
+		printf("  Name: %*.*s\n", nameLen, nameLen,
+		       (const char *)(fru_data + offset));
+
+		offset += nameLen;
+
+		pRecordData = &fru_data[offset];
+
+		printf("  Record Version: %d\n", version);
+		if (version == 0) {
+			printf("  Version: %*.*s\n",
+			       OEM_KONTRON_FIELD_SIZE,
+			       OEM_KONTRON_FIELD_SIZE,
+			       ((tOemKontronInformationRecordV0 *)pRecordData)->field1);
+			printf("  Build Date: %*.*s\n",
+			       OEM_KONTRON_FIELD_SIZE,
+			       OEM_KONTRON_FIELD_SIZE,
+			       ((tOemKontronInformationRecordV0 *)pRecordData)->field2);
+			printf("  Update Date: %*.*s\n",
+			       OEM_KONTRON_FIELD_SIZE,
+			       OEM_KONTRON_FIELD_SIZE,
+			       ((tOemKontronInformationRecordV0 *)pRecordData)->field3);
+			printf("  Checksum: %*.*s\n\n",
+			       OEM_KONTRON_FIELD_SIZE,
+			       OEM_KONTRON_FIELD_SIZE,
+			       ((tOemKontronInformationRecordV0 *)pRecordData)->crc32);
+			offset += sizeof(tOemKontronInformationRecordV0);
+			offset++;
+		} else if (version == 1) {
+			printf("  Version: %*.*s\n",
+			       OEM_KONTRON_VERSION_FIELD_SIZE,
+			       OEM_KONTRON_VERSION_FIELD_SIZE,
+			       ((tOemKontronInformationRecordV1 *)pRecordData)->field1);
+			printf("  Build Date: %*.*s\n",
+			       OEM_KONTRON_FIELD_SIZE,
+			       OEM_KONTRON_FIELD_SIZE,
+			       ((tOemKontronInformationRecordV1 *)pRecordData)->field2);
+			printf("  Update Date: %*.*s\n",
+			       OEM_KONTRON_FIELD_SIZE,
+			       OEM_KONTRON_FIELD_SIZE,
+			       ((tOemKontronInformationRecordV1 *)pRecordData)->field3);
+			printf("  Checksum: %*.*s\n\n",
+			       OEM_KONTRON_FIELD_SIZE,
+			       OEM_KONTRON_FIELD_SIZE,
+			       ((tOemKontronInformationRecordV1 *)pRecordData)->crc32);
+			offset += sizeof(tOemKontronInformationRecordV1);
+			offset++;
+		} else {
+			printf("  Unsupported version %d\n", version);
 		}
 	}
 }
 
-static int ipmi_fru_oemkontron_edit( int argc, char ** argv,uint8_t * fru_data,
+static
+bool
+ipmi_fru_oemkontron_edit( int argc, char ** argv,uint8_t * fru_data,
 												int off,int len,
 												struct fru_multirec_header *h,
 												struct fru_multirec_oem_header *oh)
 {
-	static int badParams=FALSE;
-	int hasChanged = FALSE;
+	static bool badParams=false;
+	bool hasChanged = false;
 	int start = off;
 	int offset = start;
 	int length = len;
@@ -1706,9 +1749,9 @@ static int ipmi_fru_oemkontron_edit( int argc, char ** argv,uint8_t * fru_data,
 	if(!badParams){
 		/* the 'OEM' field is already checked in caller */
 		if( argc > OEM_KONTRON_SUBCOMMAND_ARG_POS ){
-			if(strncmp("oem", argv[OEM_KONTRON_SUBCOMMAND_ARG_POS],3)){
+			if(strcmp("oem", argv[OEM_KONTRON_SUBCOMMAND_ARG_POS])){
 				printf("usage: fru edit <id> <oem> <args...>\n");
-				badParams = TRUE;
+				badParams = true;
 				return hasChanged;
 			}
 		}
@@ -1716,14 +1759,14 @@ static int ipmi_fru_oemkontron_edit( int argc, char ** argv,uint8_t * fru_data,
 			printf("usage: oem <iana> <recordid> <format> <args...>\n");
 			printf("usage: oem 15000 3 0 <name> <instance> <field1>"\
 					" <field2> <field3> <crc32>\n");
-			badParams = TRUE;
+			badParams = true;
 			return hasChanged;
 		}
 		if (str2uchar(argv[OEM_KONTRON_RECORDID_ARG_POS], &record_id) != 0) {
 			lprintf(LOG_ERR,
 					"Record ID argument '%s' is either invalid or out of range.",
 					argv[OEM_KONTRON_RECORDID_ARG_POS]);
-			badParams = TRUE;
+			badParams = true;
 			return hasChanged;
 		}
 		if (record_id == OEM_KONTRON_INFORMATION_RECORD) {
@@ -1732,7 +1775,7 @@ static int ipmi_fru_oemkontron_edit( int argc, char ** argv,uint8_t * fru_data,
 					(strlen(argv[i]) != OEM_KONTRON_VERSION_FIELD_SIZE)) {
 					printf("error: version fields must have %d characters\n",
 										OEM_KONTRON_FIELD_SIZE);
-					badParams = TRUE;
+					badParams = true;
 					return hasChanged;
 				}
 			}
@@ -1749,7 +1792,7 @@ static int ipmi_fru_oemkontron_edit( int argc, char ** argv,uint8_t * fru_data,
 				lprintf(LOG_ERR,
 						"Format argument '%s' is either invalid or out of range.",
 						argv[OEM_KONTRON_FORMAT_ARG_POS]);
-				badParams = TRUE;
+				badParams = true;
 				return hasChanged;
 			}
 
@@ -1767,7 +1810,7 @@ static int ipmi_fru_oemkontron_edit( int argc, char ** argv,uint8_t * fru_data,
 					lprintf(LOG_ERR,
 							"Instance argument '%s' is either invalid or out of range.",
 							argv[OEM_KONTRON_INSTANCE_ARG_POS]);
-					badParams = TRUE;
+					badParams = true;
 					return hasChanged;
 				}
 
@@ -1830,7 +1873,7 @@ static int ipmi_fru_oemkontron_edit( int argc, char ** argv,uint8_t * fru_data,
 							}
 
 							matchInstance++;
-							hasChanged = TRUE;
+							hasChanged = true;
 						}
 						else if(!strncmp((char *)argv[OEM_KONTRON_NAME_ARG_POS],
 							(const char *)(fru_data+offset), nameLen)){
@@ -1906,12 +1949,14 @@ static int ipmi_fru_oemkontron_edit( int argc, char ** argv,uint8_t * fru_data,
 * returns: TRUE if data changed
 * returns: FALSE if data not changed
 */
-static int ipmi_fru_picmg_ext_edit(uint8_t * fru_data,
+static
+bool
+ipmi_fru_picmg_ext_edit(uint8_t * fru_data,
 												int off,int len,
 												struct fru_multirec_header *h,
 												struct fru_multirec_oem_header *oh)
 {
-	int hasChanged = FALSE;
+	bool hasChanged = false;
 	int start = off;
 	int offset = start;
 	int length = len;
@@ -1936,7 +1981,7 @@ static int ipmi_fru_picmg_ext_edit(uint8_t * fru_data,
 					max_current |= fru_data[++index]<<8;
 					printf("      New Maximum Internal Current(@12V): %.2f A (0x%02x)\n",
 								(float)max_current / 10.0f, max_current);
-					hasChanged = TRUE;
+					hasChanged = true;
 
 				}
 
@@ -1974,7 +2019,7 @@ static int ipmi_fru_picmg_ext_edit(uint8_t * fru_data,
 
 					printf("      New Current draw(@12V): %.2f A (0x%02x)\n",
 								(float)current / 10.0f, current);
-					hasChanged = TRUE;
+					hasChanged = true;
 				}
 			}
 			break;
@@ -2885,11 +2930,11 @@ __ipmi_fru_print(struct ipmi_intf * intf, uint8_t id)
 	req.msg.data_len = 1;
 
 	rsp = intf->sendrecv(intf, &req);
-	if (rsp == NULL) {
+	if (!rsp) {
 		printf(" Device not present (No Response)\n");
 		return -1;
 	}
-	if (rsp->ccode > 0) {
+	if (rsp->ccode) {
 		printf(" Device not present (%s)\n",
 			val2str(rsp->ccode, completion_code_vals));
 		return -1;
@@ -2922,11 +2967,11 @@ __ipmi_fru_print(struct ipmi_intf * intf, uint8_t id)
 	req.msg.data_len = 4;
 
 	rsp = intf->sendrecv(intf, &req);
-	if (rsp == NULL) {
+	if (!rsp) {
 		printf(" Device not present (No Response)\n");
 		return 1;
 	}
-	if (rsp->ccode > 0) {
+	if (rsp->ccode) {
 		printf(" Device not present (%s)\n",
 				val2str(rsp->ccode, completion_code_vals));
 		return 1;
@@ -3003,7 +3048,7 @@ ipmi_fru_print(struct ipmi_intf * intf, struct sdr_record_fru_locator * fru)
 	uint32_t save_channel;
 	int rc = 0;
 
-	if (fru == NULL)
+	if (!fru)
 		return __ipmi_fru_print(intf, 0);
 
 	/* Logical FRU Device
@@ -3033,7 +3078,7 @@ ipmi_fru_print(struct ipmi_intf * intf, struct sdr_record_fru_locator * fru)
 		return 0;
 
 	memset(desc, 0, sizeof(desc));
-	memcpy(desc, fru->id_string, fru->id_code & 0x01f);
+	memcpy(desc, fru->id_string, __min(fru->id_code & 0x01f, sizeof(desc)));
 	desc[fru->id_code & 0x01f] = 0;
 	printf("FRU Device Description : %s (ID %d)\n", desc, fru->device_id);
 
@@ -3101,11 +3146,11 @@ ipmi_fru_print_all(struct ipmi_intf * intf)
 	req.msg.data_len = 0;
 
 	rsp = intf->sendrecv(intf, &req);
-	if (rsp == NULL) {
+	if (!rsp) {
 		lprintf(LOG_ERR, "Get Device ID command failed");
 		return -1;
 	}
-	if (rsp->ccode > 0) {
+	if (rsp->ccode) {
 		lprintf(LOG_ERR, "Get Device ID command failed: %s",
 			val2str(rsp->ccode, completion_code_vals));
 		return -1;
@@ -3121,14 +3166,15 @@ ipmi_fru_print_all(struct ipmi_intf * intf)
 		printf("\n");
 	}
 
-	if ((itr = ipmi_sdr_start(intf, 0)) == NULL)
+	itr = ipmi_sdr_start(intf, 0);
+	if (!itr)
 		return -1;
 
 	/* Walk the SDRs looking for FRU Devices and Management Controller Devices. */
 	/* For FRU devices, print the FRU from the SDR locator record.		    */
 	/* For MC devices, issue FRU commands to the satellite controller to print  */
 	/* FRU data.								    */
-	while ((header = ipmi_sdr_get_next_header(intf, itr)) != NULL)
+	while ((header = ipmi_sdr_get_next_header(intf, itr)))
 	{
 		if (header->type == SDR_RECORD_TYPE_MC_DEVICE_LOCATOR ) {
 			/* Check the capabilities of the Management Controller Device */
@@ -3158,11 +3204,7 @@ ipmi_fru_print_all(struct ipmi_intf * intf)
 				intf->target_addr = save_addr;
 			}
 
-			if (mc) {
-				free(mc);
-				mc = NULL;
-			}
-
+			free_n(&mc);
 			continue;
 		}
 
@@ -3172,19 +3214,15 @@ ipmi_fru_print_all(struct ipmi_intf * intf)
 		/* Print the FRU from the SDR locator record. */
 		fru = (struct sdr_record_fru_locator *)
 			ipmi_sdr_get_record(intf, header, itr);
-		if (fru == NULL || !fru->logical) {
-			if (fru) {
-				free(fru);
-				fru = NULL;
-			}
+		if (!fru || !fru->logical) {
+			free_n(&fru);
 			continue;
 		}
 		rc = ipmi_fru_print(intf, fru);
-		free(fru);
-		fru = NULL;
+		free_n(&fru);
 	}
 
-	ipmi_sdr_end(intf, itr);
+	ipmi_sdr_end(itr);
 
 	return rc;
 }
@@ -3224,8 +3262,8 @@ ipmi_fru_read_to_bin(struct ipmi_intf * intf,
 	if (!rsp)
 		return;
 
-	if (rsp->ccode > 0) {
-		if (rsp->ccode == 0xc3)
+	if (rsp->ccode) {
+		if (rsp->ccode == IPMI_CC_TIMEOUT)
 			printf ("  Timeout accessing FRU info. (Device not present?)\n");
 		return;
 	}
@@ -3240,7 +3278,7 @@ ipmi_fru_read_to_bin(struct ipmi_intf * intf,
 	}
 
 	pFruBuf = malloc(fru.size);
-	if (pFruBuf != NULL) {
+	if (pFruBuf) {
 		printf("Fru Size         : %d bytes\n",fru.size);
 		read_fru_area(intf, &fru, fruId, 0, fru.size, pFruBuf);
 	} else {
@@ -3248,7 +3286,7 @@ ipmi_fru_read_to_bin(struct ipmi_intf * intf,
 		return;
 	}
 
-	if(pFruBuf != NULL)
+	if(pFruBuf)
 	{
 		FILE * pFile;
 		pFile = fopen(pFileName,"wb");
@@ -3257,14 +3295,12 @@ ipmi_fru_read_to_bin(struct ipmi_intf * intf,
 			printf("Done\n");
 		} else {
 			lprintf(LOG_ERR, "Error opening file %s\n", pFileName);
-			free(pFruBuf);
-			pFruBuf = NULL;
+			free_n(&pFruBuf);
 			return;
 		}
 		fclose(pFile);
 	}
-	free(pFruBuf);
-	pFruBuf = NULL;
+	free_n(&pFruBuf);
 }
 
 static void
@@ -3293,7 +3329,7 @@ ipmi_fru_write_from_bin(struct ipmi_intf * intf,
 		return;
 
 	if (rsp->ccode) {
-		if (rsp->ccode == 0xc3)
+		if (rsp->ccode == IPMI_CC_TIMEOUT)
 			printf("  Timeout accessing FRU info. (Device not present?)\n");
 		return;
 	}
@@ -3308,13 +3344,13 @@ ipmi_fru_write_from_bin(struct ipmi_intf * intf,
 	}
 
 	pFruBuf = malloc(fru.size);
-	if (pFruBuf == NULL) {
+	if (!pFruBuf) {
 		lprintf(LOG_ERR, "Cannot allocate %d bytes\n", fru.size);
 		return;
 	}
 
 		pFile = fopen(pFileName, "rb");
-		if (pFile != NULL) {
+		if (pFile) {
 			len = fread(pFruBuf, 1, fru.size, pFile);
 			printf("Fru Size         : %d bytes\n", fru.size);
 			printf("Size to Write    : %d bytes\n", len);
@@ -3328,13 +3364,12 @@ ipmi_fru_write_from_bin(struct ipmi_intf * intf,
 			lprintf(LOG_INFO,"Done");
 		}
 
-	free(pFruBuf);
-	pFruBuf = NULL;
+	free_n(&pFruBuf);
 }
 
 /* ipmi_fru_write_help() - print help text for 'write'
  *
- * retruns void
+ * returns void
  */
 void
 ipmi_fru_write_help()
@@ -3411,11 +3446,11 @@ ipmi_fru_edit_multirec(struct ipmi_intf * intf, uint8_t id ,
 	req.msg.data_len = 1;
 
 	rsp = intf->sendrecv(intf, &req);
-	if (rsp == NULL) {
+	if (!rsp) {
 		printf(" Device not present (No Response)\n");
 		return -1;
 	}
-	if (rsp->ccode > 0) {
+	if (rsp->ccode) {
 		printf(" Device not present (%s)\n",
 			val2str(rsp->ccode, completion_code_vals));
 		return -1;
@@ -3446,7 +3481,7 @@ ipmi_fru_edit_multirec(struct ipmi_intf * intf, uint8_t id ,
 
 		memset(&fru, 0, sizeof(fru));
 		fru_data = malloc(fru.size + 1);
-		if (fru_data == NULL) {
+		if (!fru_data) {
 			lprintf(LOG_ERR, " Out of memory!");
 			return -1;
 		}
@@ -3480,7 +3515,7 @@ ipmi_fru_edit_multirec(struct ipmi_intf * intf, uint8_t id ,
 				if( argc <=2 ) {
 					suppliedIana =  IPMI_OEM_PICMG;
 				}  else {
-					if( !strncmp( argv[2] , "oem" , 3 )) {
+					if( !strcmp( argv[2] , "oem")) {
 						/* Expect IANA number next */
 						if( argc <= 3 ) {
 							lprintf(LOG_ERR, "oem iana <record> <format> [<args>]");
@@ -3532,8 +3567,7 @@ ipmi_fru_edit_multirec(struct ipmi_intf * intf, uint8_t id ,
 			i += h->len + sizeof (struct fru_multirec_header);
 		} while (!(h->format & 0x80) && (error != 1));
 
-		free(fru_data);
-		fru_data = NULL;
+		free_n(&fru_data);
 	}
 	return 0;
 }
@@ -3616,11 +3650,11 @@ ipmi_fru_get_multirec(struct ipmi_intf * intf, uint8_t id ,
 	req.msg.data_len = 1;
 
 	rsp = intf->sendrecv(intf, &req);
-	if (rsp == NULL) {
+	if (!rsp) {
 		printf(" Device not present (No Response)\n");
 		return -1;
 	}
-	if (rsp->ccode > 0) {
+	if (rsp->ccode) {
 		printf(" Device not present (%s)\n",
 			val2str(rsp->ccode, completion_code_vals));
 		return -1;
@@ -3650,7 +3684,7 @@ ipmi_fru_get_multirec(struct ipmi_intf * intf, uint8_t id ,
 		i = last_off = offset;
 
 		fru_data = malloc(fru.size + 1);
-		if (fru_data == NULL) {
+		if (!fru_data) {
 			lprintf(LOG_ERR, " Out of memory!");
 			return -1;
 		}
@@ -3679,7 +3713,7 @@ ipmi_fru_get_multirec(struct ipmi_intf * intf, uint8_t id ,
 
 				uint32_t suppliedIana = 0 ;
 				/* Now makes sure this is really PICMG record */
-				if( !strncmp( argv[2] , "oem" , 3 )) {
+				if( !strcmp( argv[2] , "oem")) {
 					/* Expect IANA number next */
 					if( argc <= 3 ) {
 						lprintf(LOG_ERR, "oem iana <record> <format>");
@@ -3701,15 +3735,15 @@ ipmi_fru_get_multirec(struct ipmi_intf * intf, uint8_t id ,
 				if( suppliedIana == iana ) {
 					lprintf(LOG_DEBUG, "Matching record found" );
 
-				if( iana == IPMI_OEM_KONTRON ) {
-						ipmi_fru_oemkontron_get( argc,argv,fru_data,
-						i + sizeof(struct fru_multirec_header),
-						h->len, h, oh );
+					if( iana == IPMI_OEM_KONTRON ) {
+						ipmi_fru_oemkontron_get(argc, argv, fru_data,
+									i + sizeof(struct fru_multirec_header),
+									oh);
 					}
 					/* FIXME: Add OEM record support here */
 					else{
 						printf("  OEM IANA (%s) Record not supported in this mode\n",
-															val2str( iana,  ipmi_oem_info));
+						       val2str( iana,  ipmi_oem_info));
 						error = 1;
 					}
 				}
@@ -3717,16 +3751,16 @@ ipmi_fru_get_multirec(struct ipmi_intf * intf, uint8_t id ,
 			i += h->len + sizeof (struct fru_multirec_header);
 		} while (!(h->format & 0x80) && (error != 1));
 
-		free(fru_data);
-		fru_data = NULL;
+		free_n(&fru_data);
 	}
 	return 0;
 }
 
-static int
-ipmi_fru_upg_ekeying(struct ipmi_intf * intf,
-			char * pFileName,
-			uint8_t fruId)
+#define ERR_EXIT do { rc = -1; goto exit; } while(0)
+
+static
+int
+ipmi_fru_upg_ekeying(struct ipmi_intf *intf, char *pFileName, uint8_t fruId)
 {
 	struct fru_info fruInfo = {0};
 	uint8_t *buf = NULL;
@@ -3734,59 +3768,50 @@ ipmi_fru_upg_ekeying(struct ipmi_intf * intf,
 	uint32_t fruMultiRecSize = 0;
 	uint32_t offFileMultiRec = 0;
 	uint32_t fileMultiRecSize = 0;
-	if (pFileName == NULL) {
+	int rc = 0;
+
+	if (!pFileName) {
 		lprintf(LOG_ERR, "File expected, but none given.");
-		return (-1);
+		ERR_EXIT;
 	}
 	if (ipmi_fru_get_multirec_location_from_fru(intf, fruId, &fruInfo,
 							&offFruMultiRec, &fruMultiRecSize) != 0) {
 		lprintf(LOG_ERR, "Failed to get multirec location from FRU.");
-		return (-1);
+		ERR_EXIT;
 	}
 	lprintf(LOG_DEBUG, "FRU Size        : %lu\n", fruMultiRecSize);
 	lprintf(LOG_DEBUG, "Multi Rec offset: %lu\n", offFruMultiRec);
 	if (ipmi_fru_get_multirec_size_from_file(pFileName, &fileMultiRecSize,
 				&offFileMultiRec) != 0) {
 		lprintf(LOG_ERR, "Failed to get multirec size from file '%s'.", pFileName);
-		return (-1);
+		ERR_EXIT;
 	}
 	buf = malloc(fileMultiRecSize);
-	if (buf == NULL) {
+	if (!buf) {
 		lprintf(LOG_ERR, "ipmitool: malloc failure");
-		return (-1);
+		ERR_EXIT;
 	}
 	if (ipmi_fru_get_multirec_from_file(pFileName, buf, fileMultiRecSize,
 				offFileMultiRec) != 0) {
 		lprintf(LOG_ERR, "Failed to get multirec from file '%s'.", pFileName);
-		if (buf != NULL) {
-			free(buf);
-			buf = NULL;
-		}
-		return (-1);
+		ERR_EXIT;
 	}
 	if (ipmi_fru_get_adjust_size_from_buffer(buf, &fileMultiRecSize) != 0) {
 		lprintf(LOG_ERR, "Failed to adjust size from buffer.");
-		if (buf != NULL) {
-			free(buf);
-			buf = NULL;
-		}
-		return (-1);
+		ERR_EXIT;
 	}
 	if (write_fru_area(intf, &fruInfo, fruId, 0, offFruMultiRec,
 				fileMultiRecSize, buf) != 0) {
 		lprintf(LOG_ERR, "Failed to write FRU area.");
-		if (buf != NULL) {
-			free(buf);
-			buf = NULL;
-		}
-		return (-1);
-	}
-	if (buf != NULL) {
-		free(buf);
-		buf = NULL;
+		ERR_EXIT;
 	}
+
 	lprintf(LOG_INFO, "Done upgrading Ekey.");
-	return 0;
+
+exit:
+	free_n(&buf);
+
+	return rc;
 }
 
 /* ipmi_fru_upgekey_help - print help text for 'upgEkey'
@@ -3834,7 +3859,7 @@ ipmi_fru_get_multirec_size_from_file(char * pFileName,
 		return -1;
 	}
 
-	/* Retreive length */
+	/* Retrieve length */
 	if (((header.offset.internal * 8) > (header.offset.internal * 8)) &&
 		((header.offset.internal * 8) < end))
 		end = (header.offset.internal * 8);
@@ -3882,7 +3907,7 @@ ipmi_fru_get_adjust_size_from_buffer(uint8_t * fru_data, uint32_t *pSize)
 		}
 		if (checksum != 0) {
 			lprintf(LOG_ERR, "Bad checksum in Multi Records");
-			status = (-1);
+			status = -1;
 			if (verbose) {
 				printf("--> FAIL");
 			}
@@ -3912,9 +3937,9 @@ ipmi_fru_get_multirec_from_file(char * pFileName, uint8_t * pBufArea,
 {
 	FILE * pFile;
 	uint32_t len = 0;
-	if (pFileName == NULL) {
+	if (!pFileName) {
 		lprintf(LOG_ERR, "Invalid file name given.");
-		return (-1);
+		return -1;
 	}
 	
 	errno = 0;
@@ -3922,21 +3947,21 @@ ipmi_fru_get_multirec_from_file(char * pFileName, uint8_t * pBufArea,
 	if (!pFile) {
 		lprintf(LOG_ERR, "Error opening file '%s': %i -> %s.", pFileName, errno,
 				strerror(errno));
-		return (-1);
+		return -1;
 	}
 	errno = 0;
 	if (fseek(pFile, offset, SEEK_SET) != 0) {
 		lprintf(LOG_ERR, "Failed to seek in file '%s': %i -> %s.", pFileName, errno,
 				strerror(errno));
 		fclose(pFile);
-		return (-1);
+		return -1;
 	}
 	len = fread(pBufArea, size, 1, pFile);
 	fclose(pFile);
 
 	if (len != 1) {
 		lprintf(LOG_ERR, "Error in file '%s'.", pFileName);
-		return (-1);
+		return -1;
 	}
 	return 0;
 }
@@ -3971,8 +3996,8 @@ ipmi_fru_get_multirec_location_from_fru(struct ipmi_intf * intf,
 		return -1;
 	}
 
-	if (rsp->ccode > 0) {
-		if (rsp->ccode == 0xc3)
+	if (rsp->ccode) {
+		if (rsp->ccode == IPMI_CC_TIMEOUT)
 			printf ("  Timeout accessing FRU info. (Device not present?)\n");
 		else
 			printf ("   CCODE = 0x%02x\n", rsp->ccode);
@@ -4003,8 +4028,8 @@ ipmi_fru_get_multirec_location_from_fru(struct ipmi_intf * intf,
 
 	if (!rsp)
 		return -1;
-	if (rsp->ccode > 0) {
-		if (rsp->ccode == 0xc3)
+	if (rsp->ccode) {
+		if (rsp->ccode == IPMI_CC_TIMEOUT)
 			printf ("  Timeout while reading FRU data. (Device not present?)\n");
 		return -1;
 	}
@@ -4021,7 +4046,7 @@ ipmi_fru_get_multirec_location_from_fru(struct ipmi_intf * intf,
 
 	end = pFruInfo->size;
 
-	/* Retreive length */
+	/* Retrieve length */
 	if (((header.offset.internal * 8) > (header.offset.internal * 8)) &&
 		((header.offset.internal * 8) < end))
 		end = (header.offset.internal * 8);
@@ -4044,7 +4069,7 @@ ipmi_fru_get_multirec_location_from_fru(struct ipmi_intf * intf,
 	return 0;
 }
 
-/* ipmi_fru_get_internal_use_offset -  Retreive internal use offset
+/* ipmi_fru_get_internal_use_offset -  Retrieve internal use offset
 *
 * @intf:   ipmi interface
 * @id:     fru id
@@ -4085,17 +4110,16 @@ ipmi_fru_get_internal_use_info(  struct ipmi_intf * intf,
 	req.msg.data_len = 1;
 
 	rsp = intf->sendrecv(intf, &req);
-	if (rsp == NULL) {
+	if (!rsp) {
 		printf(" Device not present (No Response)\n");
 		return -1;
 	}
-	if (rsp->ccode > 0) {
+	if (rsp->ccode) {
 		printf(" Device not present (%s)\n",
 			val2str(rsp->ccode, completion_code_vals));
 		return -1;
 	}
 
-	memset(&fru, 0, sizeof(fru));
 	fru->size = (rsp->data[1] << 8) | rsp->data[0];
 	fru->access = rsp->data[2] & 0x1;
 
@@ -4122,11 +4146,11 @@ ipmi_fru_get_internal_use_info(  struct ipmi_intf * intf,
 	req.msg.data_len = 4;
 
 	rsp = intf->sendrecv(intf, &req);
-	if (rsp == NULL) {
+	if (!rsp) {
 		printf(" Device not present (No Response)\n");
 		return 1;
 	}
-	if (rsp->ccode > 0) {
+	if (rsp->ccode) {
 		printf(" Device not present (%s)\n",
 				val2str(rsp->ccode, completion_code_vals));
 		return 1;
@@ -4265,7 +4289,7 @@ ipmi_fru_read_internal_use(struct ipmi_intf * intf, uint8_t id, char * pFileName
 
 			if(rc == 0)
 			{
-				if(pFileName == NULL)
+				if(!pFileName)
 				{
 					uint16_t counter;
 					for(counter = 0; counter < size; counter ++)
@@ -4287,8 +4311,7 @@ ipmi_fru_read_internal_use(struct ipmi_intf * intf, uint8_t id, char * pFileName
 					else
 					{
 						lprintf(LOG_ERR, "Error opening file %s\n", pFileName);
-						free(frubuf);
-						frubuf = NULL;
+						free_n(&frubuf);
 						return -1;
 					}
 					fclose(pFile);
@@ -4296,8 +4319,7 @@ ipmi_fru_read_internal_use(struct ipmi_intf * intf, uint8_t id, char * pFileName
 			}
 			printf("\n");
 
-			free(frubuf);
-			frubuf = NULL;
+			free_n(&frubuf);
 		}
 
 	}
@@ -4340,7 +4362,7 @@ ipmi_fru_write_internal_use(struct ipmi_intf * intf, uint8_t id, char * pFileNam
 
 		if(fp)
 		{
-			/* Retreive file length, check if it's fits the Eeprom Size */
+			/* Retrieve file length, check if it's fits the Eeprom Size */
 			fseek(fp, 0 ,SEEK_END);
 			fileLength = ftell(fp);
 
@@ -4380,8 +4402,7 @@ ipmi_fru_write_internal_use(struct ipmi_intf * intf, uint8_t id, char * pFileNam
 					lprintf(LOG_ERR, "Unable to read file: %i\n", fru_read_size);
 				}
 
-				free(frubuf);
-				frubuf = NULL;
+				free_n(&frubuf);
 			}
 			fclose(fp);
 			fp = NULL;
@@ -4403,42 +4424,43 @@ ipmi_fru_main(struct ipmi_intf * intf, int argc, char ** argv)
 	if (argc < 1) {
 		rc = ipmi_fru_print_all(intf);
 	}
-	else if (strncmp(argv[0], "help", 4) == 0) {
+	else if (!strcmp(argv[0], "help")) {
 		ipmi_fru_help();
 		return 0;
 	}
-	else if (strncmp(argv[0], "print", 5) == 0 ||
-		strncmp(argv[0], "list", 4) == 0) {
+	else if (!strcmp(argv[0], "print")
+	         || !strcmp(argv[0], "list"))
+	{
 		if (argc > 1) {
-			if (strcmp(argv[1], "help") == 0) {
+			if (!strcmp(argv[1], "help")) {
 				lprintf(LOG_NOTICE, "fru print [fru id] - print information about FRU(s)");
 				return 0;
 			}
 
 			if (is_fru_id(argv[1], &fru_id) != 0)
-				return (-1);
+				return -1;
 
 			rc = __ipmi_fru_print(intf, fru_id);
 		} else {
 			rc = ipmi_fru_print_all(intf);
 		}
 	}
-	else if (!strncmp(argv[0], "read", 5)) {
-		if (argc > 1 && strcmp(argv[1], "help") == 0) {
+	else if (!strcmp(argv[0], "read")) {
+		if (argc > 1 && !strcmp(argv[1], "help")) {
 			ipmi_fru_read_help();
 			return 0;
 		} else if (argc < 3) {
 			lprintf(LOG_ERR, "Not enough parameters given.");
 			ipmi_fru_read_help();
-			return (-1);
+			return -1;
 		}
 
 		if (is_fru_id(argv[1], &fru_id) != 0)
-			return (-1);
+			return -1;
 
 		/* There is a file name in the parameters */
 		if (is_valid_filename(argv[2]) != 0)
-				return (-1);
+			return -1;
 
 		if (verbose) {
 			printf("FRU ID           : %d\n", fru_id);
@@ -4447,22 +4469,22 @@ ipmi_fru_main(struct ipmi_intf * intf, int argc, char ** argv)
 		/* TODO - rc is missing */
 		ipmi_fru_read_to_bin(intf, argv[2], fru_id);
 	}
-	else if (!strncmp(argv[0], "write", 5)) {
-		if (argc > 1 && strcmp(argv[1], "help") == 0) {
+	else if (!strcmp(argv[0], "write")) {
+		if (argc > 1 && !strcmp(argv[1], "help")) {
 			ipmi_fru_write_help();
 			return 0;
 		} else if (argc < 3) {
 			lprintf(LOG_ERR, "Not enough parameters given.");
 			ipmi_fru_write_help();
-			return (-1);
+			return -1;
 		}
 
 		if (is_fru_id(argv[1], &fru_id) != 0)
-			return (-1);
+			return -1;
 
 		/* There is a file name in the parameters */
 		if (is_valid_filename(argv[2]) != 0)
-				return (-1);
+			return -1;
 
 		if (verbose) {
 			printf("FRU ID           : %d\n", fru_id);
@@ -4471,67 +4493,67 @@ ipmi_fru_main(struct ipmi_intf * intf, int argc, char ** argv)
 		/* TODO - rc is missing */
 		ipmi_fru_write_from_bin(intf, argv[2], fru_id);
 	}
-	else if (!strncmp(argv[0], "upgEkey", 7)) {
-		if (argc > 1 && strcmp(argv[1], "help") == 0) {
+	else if (!strcmp(argv[0], "upgEkey")) {
+		if (argc > 1 && !strcmp(argv[1], "help")) {
 			ipmi_fru_upgekey_help();
 			return 0;
 		} else if (argc < 3) {
 			lprintf(LOG_ERR, "Not enough parameters given.");
 			ipmi_fru_upgekey_help();
-			return (-1);
+			return -1;
 		}
 
 		if (is_fru_id(argv[1], &fru_id) != 0)
-			return (-1);
+			return -1;
 
 		/* There is a file name in the parameters */
 		if (is_valid_filename(argv[2]) != 0)
-				return (-1);
+			return -1;
 
 		rc = ipmi_fru_upg_ekeying(intf, argv[2], fru_id);
 	}
-	else if (!strncmp(argv[0], "internaluse", 11)) {
-		if (argc > 1 && strcmp(argv[1], "help") == 0) {
+	else if (!strcmp(argv[0], "internaluse")) {
+		if (argc > 1 && !strcmp(argv[1], "help")) {
 			ipmi_fru_internaluse_help();
 			return 0;
 		}
 
-		if ( (argc >= 3) && (!strncmp(argv[2], "info", 4)) ) {
+		if ( (argc >= 3) && (!strcmp(argv[2], "info")) ) {
 
 			if (is_fru_id(argv[1], &fru_id) != 0)
-				return (-1);
+				return -1;
 
 			rc = ipmi_fru_info_internal_use(intf, fru_id);
 		}
-		else if ( (argc >= 3) && (!strncmp(argv[2], "print", 5)) ) {
+		else if ( (argc >= 3) && (!strcmp(argv[2], "print")) ) {
 
 			if (is_fru_id(argv[1], &fru_id) != 0)
-				return (-1);
+				return -1;
 
 			rc = ipmi_fru_read_internal_use(intf, fru_id, NULL);
 		}
-		else if ( (argc >= 4) && (!strncmp(argv[2], "read", 4)) ) {
+		else if ( (argc >= 4) && (!strcmp(argv[2], "read")) ) {
 
 			if (is_fru_id(argv[1], &fru_id) != 0)
-				return (-1);
+				return -1;
 
 			/* There is a file name in the parameters */
 			if (is_valid_filename(argv[3]) != 0)
-					return (-1);
+				return -1;
 
 			lprintf(LOG_DEBUG, "FRU ID           : %d", fru_id);
 			lprintf(LOG_DEBUG, "FRU File         : %s", argv[3]);
 
 			rc = ipmi_fru_read_internal_use(intf, fru_id, argv[3]);
 		}
-		else if ( (argc >= 4) && (!strncmp(argv[2], "write", 5)) ) {
+		else if ( (argc >= 4) && (!strcmp(argv[2], "write")) ) {
 
 			if (is_fru_id(argv[1], &fru_id) != 0)
-				return (-1);
+				return -1;
 
 			/* There is a file name in the parameters */
 			if (is_valid_filename(argv[3]) != 0)
-					return (-1);
+				return -1;
 
 			lprintf(LOG_DEBUG, "FRU ID           : %d", fru_id);
 			lprintf(LOG_DEBUG, "FRU File         : %s", argv[3]);
@@ -4541,22 +4563,22 @@ ipmi_fru_main(struct ipmi_intf * intf, int argc, char ** argv)
 			lprintf(LOG_ERR,
 					"Either unknown command or not enough parameters given.");
 			ipmi_fru_internaluse_help();
-			return (-1);
+			return -1;
 		}
 	}
-	else if (!strncmp(argv[0], "edit", 4)) {
-		if (argc > 1 && strcmp(argv[1], "help") == 0) {
+	else if (!strcmp(argv[0], "edit")) {
+		if (argc > 1 && !strcmp(argv[1], "help")) {
 			ipmi_fru_edit_help();
 			return 0;
 		} else if (argc < 2) {
 			lprintf(LOG_ERR, "Not enough parameters given.");
 			ipmi_fru_edit_help();
-			return (-1);
+			return -1;
 		}
 		
 		if (argc >= 2) {
 			if (is_fru_id(argv[1], &fru_id) != 0)
-				return (-1);
+				return -1;
 
 			if (verbose) {
 				printf("FRU ID           : %d\n", fru_id);
@@ -4566,38 +4588,38 @@ ipmi_fru_main(struct ipmi_intf * intf, int argc, char ** argv)
 		}
 
 		if (argc >= 3) {
-			if (!strncmp(argv[2], "field", 5)) {
+			if (!strcmp(argv[2], "field")) {
 				if (argc != 6) {
 					lprintf(LOG_ERR, "Not enough parameters given.");
 					ipmi_fru_edit_help();
-					return (-1);
+					return -1;
 				}
 				rc = ipmi_fru_set_field_string(intf, fru_id, *argv[3], *argv[4],
 						(char *) argv[5]);
-			} else if (!strncmp(argv[2], "oem", 3)) {
+			} else if (!strcmp(argv[2], "oem")) {
 				rc = ipmi_fru_edit_multirec(intf, fru_id, argc, argv);
 			} else {
 				lprintf(LOG_ERR, "Invalid command: %s", argv[2]);
 				ipmi_fru_edit_help();
-				return (-1);
+				return -1;
 			}
 		} else {
 			rc = ipmi_fru_edit_multirec(intf, fru_id, argc, argv);
 		}
 	}
-	else if (!strncmp(argv[0], "get", 4)) {
-		if (argc > 1 && (strncmp(argv[1], "help", 4) == 0)) {
+	else if (!strcmp(argv[0], "get")) {
+		if (argc > 1 && (!strcmp(argv[1], "help"))) {
 			ipmi_fru_get_help();
 			return 0;
 		} else if (argc < 2) {
 			lprintf(LOG_ERR, "Not enough parameters given.");
 			ipmi_fru_get_help();
-			return (-1);
+			return -1;
 		}
 
 		if (argc >= 2) {
 			if (is_fru_id(argv[1], &fru_id) != 0)
-				return (-1);
+				return -1;
 
 			if (verbose) {
 				printf("FRU ID           : %d\n", fru_id);
@@ -4607,12 +4629,12 @@ ipmi_fru_main(struct ipmi_intf * intf, int argc, char ** argv)
 		}
 
 		if (argc >= 3) {
-			if (!strncmp(argv[2], "oem", 3)) {
+			if (!strcmp(argv[2], "oem")) {
 				rc = ipmi_fru_get_multirec(intf, fru_id, argc, argv);
 			} else {
 				lprintf(LOG_ERR, "Invalid command: %s", argv[2]);
 				ipmi_fru_get_help();
-				return (-1);
+				return -1;
 			}
 		} else {
 			rc = ipmi_fru_get_multirec(intf, fru_id, argc, argv);
@@ -4621,7 +4643,7 @@ ipmi_fru_main(struct ipmi_intf * intf, int argc, char ** argv)
 	else {
 		lprintf(LOG_ERR, "Invalid FRU command: %s", argv[0]);
 		ipmi_fru_help();
-		return (-1);
+		return -1;
 	}
 
 	return rc;
@@ -4668,15 +4690,15 @@ f_type, uint8_t f_index, char *f_string)
 	req.msg.data_len = 1;
 
 	rsp = intf->sendrecv(intf, &req);
-	if (rsp == NULL) {
+	if (!rsp) {
 		printf(" Device not present (No Response)\n");
-		rc = (-1);
+		rc = -1;
 		goto ipmi_fru_set_field_string_out;
 	}
-	if (rsp->ccode > 0) {
+	if (rsp->ccode) {
 		printf(" Device not present (%s)\n",
 			val2str(rsp->ccode, completion_code_vals));
-		rc = (-1);
+		rc = -1;
 		goto ipmi_fru_set_field_string_out;
 	}
 
@@ -4686,7 +4708,7 @@ f_type, uint8_t f_index, char *f_string)
 
 	if (fru.size < 1) {
 		printf(" Invalid FRU size %d", fru.size);
-		rc = (-1);
+		rc = -1;
 		goto ipmi_fru_set_field_string_out;
 	}
 	/*
@@ -4704,17 +4726,17 @@ f_type, uint8_t f_index, char *f_string)
 	req.msg.data_len = 4;
 
 	rsp = intf->sendrecv(intf, &req);
-	if (rsp == NULL)
+	if (!rsp)
 	{
 		printf(" Device not present (No Response)\n");
-		rc = (-1);
+		rc = -1;
 		goto ipmi_fru_set_field_string_out;
 	}
-	if (rsp->ccode > 0)
+	if (rsp->ccode)
 	{
 		printf(" Device not present (%s)\n",
 				val2str(rsp->ccode, completion_code_vals));
-		rc = (-1);
+		rc = -1;
 		goto ipmi_fru_set_field_string_out;
 	}
 
@@ -4723,20 +4745,17 @@ f_type, uint8_t f_index, char *f_string)
 
 	memcpy(&header, rsp->data + 1, 8);
 
-	if (header.version != 1)
-	{
+	if (header.version != 1) {
 		printf(" Unknown FRU header version 0x%02x",
 			header.version);
-		rc = (-1);
+		rc = -1;
 		goto ipmi_fru_set_field_string_out;
 	}
 
 	fru_data = malloc( fru.size );
-
-	if( fru_data == NULL )
-	{
+	if (!fru_data) {
 		printf("Out of memory!\n");
-		rc = (-1);
+		rc = -1;
 		goto ipmi_fru_set_field_string_out;
 	}
 
@@ -4766,14 +4785,14 @@ f_type, uint8_t f_index, char *f_string)
 	else
 	{
 		printf("Wrong field type.");
-		rc = (-1);
+		rc = -1;
 		goto ipmi_fru_set_field_string_out;
 	}
 	memset(fru_data, 0, fru.size);
 	if( read_fru_area(intf ,&fru, fruId, header_offset ,
 					fru_section_len , fru_data) < 0 )
 	{
-		rc = (-1);
+		rc = -1;
 		goto ipmi_fru_set_field_string_out;
 	}
 	/* Convert index from character to decimal */
@@ -4782,16 +4801,15 @@ f_type, uint8_t f_index, char *f_string)
 	/*Seek to field index */
 	for (i=0; i <= f_index; i++) {
 		fru_field_offset_tmp = fru_field_offset;
-		if (fru_area != NULL) {
-			free(fru_area);
-			fru_area = NULL;
+		if (fru_area) {
+			free_n(&fru_area);
 		}
 		fru_area = (uint8_t *) get_fru_area_str(fru_data, &fru_field_offset);
 	}
 
-	if( (fru_area == NULL )  || strlen((const char *)fru_area) == 0 ) {
+	if (!FRU_FIELD_VALID(fru_area)) {
 		printf("Field not found !\n");
-		rc = (-1);
+		rc = -1;
 		goto ipmi_fru_set_field_string_out;
 	}
 
@@ -4815,7 +4833,7 @@ f_type, uint8_t f_index, char *f_string)
 				header_offset, fru_section_len, fru_data) < 0 )
 		{
 			printf("Write to FRU data failed.\n");
-			rc = (-1);
+			rc = -1;
 			goto ipmi_fru_set_field_string_out;
 		}
 	}
@@ -4825,20 +4843,14 @@ f_type, uint8_t f_index, char *f_string)
 				ipmi_fru_set_field_string_rebuild(intf,fruId,fru,header,f_type,f_index,f_string)
 		)
 		{
-			rc = (-1);
+			rc = -1;
 			goto ipmi_fru_set_field_string_out;
 		}
 	}
 
-	ipmi_fru_set_field_string_out:
-	if (fru_data != NULL) {
-		free(fru_data);
-		fru_data = NULL;
-	}
-	if (fru_area != NULL) {
-		free(fru_area);
-		fru_area = NULL;
-	}
+ipmi_fru_set_field_string_out:
+	free_n(&fru_data);
+	free_n(&fru_area);
 
 	return rc;
 }
@@ -4894,10 +4906,9 @@ ipmi_fru_set_field_string_rebuild(struct ipmi_intf * intf, uint8_t fruId,
 
 	fru_data_new = malloc( fru.size );
 
-	if( fru_data_old == NULL || fru_data_new == NULL )
-	{
+	if (!fru_data_old || !fru_data_new) {
 		printf("Out of memory!\n");
-		rc = (-1);
+		rc = -1;
 		goto ipmi_fru_set_field_string_rebuild_out;
 	}
 
@@ -4952,7 +4963,7 @@ ipmi_fru_set_field_string_rebuild(struct ipmi_intf * intf, uint8_t fruId,
 	else
 	{
 		printf("Wrong field type.");
-		rc = (-1);
+		rc = -1;
 		goto ipmi_fru_set_field_string_rebuild_out;
 	}
 
@@ -4960,16 +4971,13 @@ ipmi_fru_set_field_string_rebuild(struct ipmi_intf * intf, uint8_t fruId,
 	3) Seek to field index */
 	for (i = 0;i <= f_index; i++) {
 		fru_field_offset_tmp = fru_field_offset;
-		if (fru_area != NULL) {
-			free(fru_area);
-			fru_area = NULL;
-		}
+		free_n(&fru_area);
 		fru_area = (uint8_t *) get_fru_area_str(fru_data_old, &fru_field_offset);
 	}
 
-	if( (fru_area == NULL )  || strlen((const char *)fru_area) == 0 ) {
+	if (!FRU_FIELD_VALID(fru_area)) {
 		printf("Field not found (1)!\n");
-		rc = (-1);
+		rc = -1;
 		goto ipmi_fru_set_field_string_rebuild_out;
 	}
 
@@ -5142,7 +5150,7 @@ ipmi_fru_set_field_string_rebuild(struct ipmi_intf * intf, uint8_t fruId,
 		*(fru_data_new + fru_field_offset_tmp) = (0xc0 + strlen(f_string));
 		memcpy(fru_data_new + fru_field_offset_tmp + 1, f_string, strlen(f_string));
 
-		/* Copy remaing bytes in section */
+		/* Copy remaining bytes in section */
 #ifdef DBG_RESIZE_FRU
 		printf("Copying remaining of sections: %d \n",
 		 (int)((fru_data_old + header_offset + fru_section_len - 1) -
@@ -5178,7 +5186,7 @@ ipmi_fru_set_field_string_rebuild(struct ipmi_intf * intf, uint8_t fruId,
 	else
 	{
 		printf( "Internal error, padding length %i (must be from 0 to 7) ", padding_len );
-		rc = (-1);
+		rc = -1;
 		goto ipmi_fru_set_field_string_rebuild_out;
 	}
 
@@ -5188,25 +5196,16 @@ ipmi_fru_set_field_string_rebuild(struct ipmi_intf * intf, uint8_t fruId,
 	if( write_fru_area( intf, &fru, fruId, 0, 0, fru.size, fru_data_new ) < 0 )
 	{
 		printf("Write to FRU data failed.\n");
-		rc = (-1);
+		rc = -1;
 		goto ipmi_fru_set_field_string_rebuild_out;
 	}
 
 	printf("Done.\n");
 
-	ipmi_fru_set_field_string_rebuild_out:
-	if (fru_area != NULL) {
-		free(fru_area);
-		fru_area = NULL;
-	}
-	if (fru_data_new != NULL) {
-		free(fru_data_new);
-		fru_data_new = NULL;
-	}
-	if (fru_data_old != NULL) {
-		free(fru_data_old);
-		fru_data_old = NULL;
-	}
+ipmi_fru_set_field_string_rebuild_out:
+	free_n(&fru_area);
+	free_n(&fru_data_new);
+	free_n(&fru_data_old);
 
 	return rc;
 }