[PATCH openpower-host-ipmi-oem 3/8] Fixes from Joel review
OpenBMC Patches
patches at stwcx.xyz
Thu Oct 22 13:52:08 AEDT 2015
From: Chris Austen <austenc at us.ibm.com>
---
Makefile | 8 +++----
ipmid-api.h | 4 ++--
oemhandler.C | 34 ++++++++++++++---------------
testit.C | 70 ++++++++++++++++++++++++------------------------------------
4 files changed, 51 insertions(+), 65 deletions(-)
diff --git a/Makefile b/Makefile
index e2c405b..e2ed670 100644
--- a/Makefile
+++ b/Makefile
@@ -6,14 +6,14 @@ TESTER_OBJ = testit.o oemhandler.o
LIB_OEM_OBJ = oemhandler.o
LIB_OEM = liboemhandler.so
-INC_FLAG += -I. -O2 --std=gnu++11
-LIB_FLAG += -rdynamic
+LDFLAGS += -rdynamic
+CXXFLAGS += -fpic -Wall
all: $(TESTER) $(LIB_OEM)
%.o: %.C
- $(CXX) -fpic -c $< $(CXXFLAGS) $(INC_FLAG) $(IPMID_PATH) -o $@
+ $(CXX) -fpic -c $< $(CXXFLAGS) -o $@
$(LIB_OEM): $(LIB_OEM_OBJ)
@@ -24,4 +24,4 @@ $(TESTER): $(TESTER_OBJ)
$(CXX) $^ $(LDFLAGS) $(LIB_FLAG) -o $@ -ldl
clean:
- rm -f $(TESTER) *.o *.so
+ $(RM) $(LIB_OEM_OBJ) $(LIB_OEM) $(TESTER_OBJ) $(TESTER)
diff --git a/ipmid-api.h b/ipmid-api.h
index ef2be38..68090f3 100644
--- a/ipmid-api.h
+++ b/ipmid-api.h
@@ -1,5 +1,5 @@
-#ifndef __HOST_IPMID_IPMI_COMMON_H__
-#define __HOST_IPMID_IPMI_COMMON_H__
+#ifndef __HOST_IPMID_API_COMMON_H__
+#define __HOST_IPMID_API_COMMON_H__
#include <stdlib.h>
// length of Completion Code and its ALWAYS _1_
diff --git a/oemhandler.C b/oemhandler.C
index c4eae21..fa1b9a2 100644
--- a/oemhandler.C
+++ b/oemhandler.C
@@ -5,8 +5,6 @@
void register_netfn_oem_partial_esel() __attribute__((constructor));
-
-
const char *g_esel_path = "/tmp/";
uint16_t g_record_id = 0x0100;
@@ -32,39 +30,41 @@ ipmi_ret_t ipmi_ibm_oem_partial_esel(ipmi_netfn_t netfn, ipmi_cmd_t cmd,
{
esel_request_t *reqptr = (esel_request_t*) request;
FILE *fp;
- char string[32];
- short offset = 0, record=0;
- unsigned short rlen;
+ short offset = 0;
+ uint8_t rlen;
ipmi_ret_t rc = IPMI_CC_OK;
- char iocmd[4];
+ char iocmd[][4] = { { "wb" }, {"rb+"} };
+ char string[64];
+ char *pio;
- strcpy(string, g_esel_path);
offset = LSMSSWAP(reqptr->offsetls, reqptr->offsetms);
- sprintf(string, "%s%s%02x%02x", string, "esel", reqptr->selrecordms, reqptr->selrecordls);
+ snprintf(string, sizeof(string), "%s%s%02x%02x", g_esel_path, "esel", reqptr->selrecordms, reqptr->selrecordls);
// Length is the number of request bytes minus the header itself.
// The header contains an extra byte to indicate the start of
// the data (so didn't need to worry about word/byte boundaries)
// hence the -1...
- rlen = ((unsigned short)*data_len) - (sizeof(esel_request_t));
+ rlen = (*data_len) - (uint8_t) (sizeof(esel_request_t));
printf("IPMI PARTIAL ESEL for %s Offset = %d Length = %d\n",
string, offset, rlen);
+
+ // Rules state for a Partial eSel that the first write of a
+ // new esel will be the sensor data record. We will use that
+ // to indicate this is a new record rather then an ofset in
+ // my next commit TODO
if (offset == 0) {
- strcpy(iocmd, "wb");
+ pio = iocmd[0];
} else {
- // I was thinking "ab+" but it appears it doesn't
- // do what fseek asks. Modify to rb+ and fseek
- // works great...
- strcpy(iocmd, "rb+");
+ pio = iocmd[1];
}
- if ((fp = fopen(string, iocmd)) != NULL) {
+ if ((fp = fopen(string, pio)) != NULL) {
fseek(fp, offset, SEEK_SET);
fwrite(reqptr+1,rlen,1,fp);
fclose(fp);
@@ -75,8 +75,8 @@ ipmi_ret_t ipmi_ibm_oem_partial_esel(ipmi_netfn_t netfn, ipmi_cmd_t cmd,
} else {
- fprintf(stderr, "Error trying to perform %s for esel%s\n",iocmd, string);
- ipmi_ret_t rc = IPMI_CC_INVALID;
+ fprintf(stderr, "Error trying to perform %s for esel%s\n",pio, string);
+ rc = IPMI_CC_INVALID;
*data_len = 0;
}
diff --git a/testit.C b/testit.C
index b563257..25e069c 100644
--- a/testit.C
+++ b/testit.C
@@ -4,6 +4,12 @@
#include "oemhandler.h"
#include <cstring>
+#include <iostream>
+#include <fstream>
+#include <vector>
+
+using namespace std;
+
const char* g_filepath = "/tmp/";
const char * getFilePath(void) { return g_filepath; }
@@ -12,56 +18,36 @@ const char * getFilePath(void) { return g_filepath; }
#define MAXRESPONSE 2
-// Returns the length of the file
-size_t checkFileSize(const char *fn) {
- FILE *fp;
- size_t len = -1;
-
- if ((fp = fopen(fn, "rb")) != NULL) {
- fseek(fp, 0, SEEK_END);
- len = ftell(fp);
- fclose(fp);
- }
-
- return len;
+// Returns the length of the file
+std::ifstream::pos_type filesize(const char* filename)
+{
+ std::ifstream in(filename, std::ifstream::ate | std::ifstream::binary);
+ return in.tellg();
}
+
// Compares a string to the data in a file.
// Returns 0 if complete match
-int compareData(const char *fn, const char *string, size_t len) {
+int compareData(const char *filename, const char *string, size_t len)
+{
- int rc = 0;
- FILE *fp;
- char fbyte;
- int i=0;
+ std::ifstream in(filename, std::ifstream::ate | std::ifstream::binary);
+ std::streamsize size = in.tellg();
- if ((fp = fopen(fn, "rb")) != NULL) {
+ std::vector<char> buffer(size);
+
+ in.read(buffer.data(), size);
+
+ if (!std::memcmp(string, buffer.data(), len))
+ return -1;
- while(fread(&fbyte, 1,1,fp) == 1) {
- if (*(string+i) != fbyte) {
- rc = -2;
- break;
- }
- i++;
- }
- fclose(fp);
-
- if (len != i) {
- rc = -3;
- }
-
- } else {
- rc = -1;
- }
-
-
-
- return rc;
+
+ return 0;
}
-void test_multiwrite(int segment, const char *pString) {
+void test_multiwrite(unsigned int segment, const char *pString) {
uint8_t request[1024];
uint8_t response[MAXRESPONSE];
@@ -87,7 +73,7 @@ void test_multiwrite(int segment, const char *pString) {
rc = ipmi_ibm_oem_partial_esel(0x3E, 0xF0, pRequest, pResponse, pLen, NULL);
if (rc != IPMI_CC_OK) { printf("Error completion code returned %d\n", rc);}
- if (len != 2) { printf("Error data buffer length failed len=%lu\n", len);}
+ if (len != 2) { printf("Error data buffer length failed len\n");}
pReqHdr->selrecordls = response[0];
pReqHdr->selrecordms = response[1];
@@ -114,7 +100,7 @@ void test_multiwrite(int segment, const char *pString) {
rc = ipmi_ibm_oem_partial_esel(0x3E, 0xF0, pRequest, pResponse, pLen, NULL);
if (rc != IPMI_CC_OK) { printf("Error completion code returned %d\n", rc);}
- if (len != 2) { printf("Error data buffer length failed len=%lu\n", len);}
+ if (len != 2) { printf("Error data buffer length failed\n");}
pReqHdr->selrecordls = response[0];
pReqHdr->selrecordms = response[1];
@@ -123,7 +109,7 @@ void test_multiwrite(int segment, const char *pString) {
}
- if (checkFileSize("/tmp/esel0100") != strlen(pString)) { printf("Error fileszie mismatch\n");}
+ if (filesize("/tmp/esel0100") != (unsigned int) strlen(pString)) { printf("Error fileszie mismatch\n");}
// /tmp/esel000 should be identical to the incoming string
rc = compareData("/tmp/esel0100",pString,strlen(pString));
--
2.6.0
More information about the openbmc
mailing list