some tests with lint

src/Makefile

@@ -6,18 +6,12 @@ LDFLAGS=-lwiringPi
 mbusgw: mbusgw.o
 	$(CC) -o $@ $(LDFLAGS) $^
 
-mbusgw.o:	mbusgw.c mbusgw.h
+.c.o:
-
-%.o : %.c
 	$(CC) $(CFLAGS) -c $<
 
 .PHONY: all
 all:    mbusgw
 
-.PHONY: install
-install: all
-	sudo cp mbusgw /usr/local/bin/
-
 .PHONY: clean
 clean:
 	-rm -f *.o mbusgw

src/mbusgw.c

@@ -32,17 +32,15 @@ typedef enum {
 } t_state;
 
 
-int serialFd;
+static bool verbose = false;
-bool verbose = false;
+static bool loopActiveFlag = false;
-bool loopActiveFlag = false;
-bool loopEnabled = false;
 
-void msleep(uint32_t t) {
+static void msleep(uint32_t t) {
-  usleep(t * 1000);
+  usleep((useconds_t)(t * 1000));
 }
 
 
-void infolog(const char *format, ...) {
+static void infolog(const char *format, ...) {
   va_list ap;
   va_start(ap, format);
   if (verbose) {
@@ -51,7 +49,7 @@ void infolog(const char *format, ...) {
   va_end(ap);
 }
 
-void errlog(const char *format, ...) {
+static void errlog(const char *format, ...) {
   va_list ap;
   va_start(ap, format);
   vfprintf(stderr, format, ap);
@@ -59,7 +57,7 @@ void errlog(const char *format, ...) {
 }
 
 
-void ledRed(bool v) {
+static void ledRed(bool v) {
   if (v) {
     digitalWrite(LED_RED, HIGH);
   } else {
@@ -67,67 +65,64 @@ void ledRed(bool v) {
   }
 }
 
-void ledGreen(bool v) {
+static void ledGreen(bool v) {
   if (v) {
     digitalWrite(LED_GREEN, HIGH);
   } else {
     digitalWrite(LED_GREEN, LOW);
   }
 }
-void frontendReset() {
+
+static void frontendReset() {
   digitalWrite(FRONTEND_RESET, LOW);
   msleep(25);
   digitalWrite(FRONTEND_RESET, HIGH);
   msleep(100);
 }
 
-void loopControl(bool v) {
+static void loopControl(bool v) {
   if (v) {
     digitalWrite(LOOP_ENABLE, HIGH);
     msleep(5);
     digitalWrite(LOOP_ENABLE, LOW);
-    loopEnabled = true;
   } else {
-    loopEnabled = false;
     digitalWrite(LOOP_DISABLE, HIGH);
     digitalWrite(LOOP_DISABLE, LOW);
   }
 }
 
-void frontendSample() {
+static void frontendSample() {
   digitalWrite(FRONTEND_SAMPLE_HOLD, LOW);
 }
 
-void frontendHold() {
+static void frontendHold() {
   digitalWrite(FRONTEND_SAMPLE_HOLD, HIGH);
 }
 
-void loopStatusISR() {
+static void loopStatusISR() {
   loopActiveFlag = digitalRead(LOOP_STATUS) == LOW;
-  if ((! loopActiveFlag) && (loopEnabled)) {
+  if (! loopActiveFlag) {
-    loopEnabled = false;
     ledRed(true);
   }
 }
 
 
-void myExit(int e) {
+static void deinit() {
   ledRed(false);
   ledGreen(false);
   loopControl(false);
   frontendSample();
-
-  exit(e);
 }
 
-void termHandler(int signum)
+static void termHandler(/*@unused@*/ int signum)
 {
    infolog("Termination requested via signal\n");
-   myExit(0);
+   deinit();
+   exit(EXIT_SUCCESS);
 }
 
 
-void init() {
+static void init() {
   infolog("Register termination handler\n");
   signal(SIGTERM, termHandler);
   signal(SIGINT, termHandler);
@@ -164,7 +159,7 @@ void init() {
   loopControl(false);
 }
 
-int openSerial(char *serialDevice, uint32_t speedNum) {
+static int openSerial(char *serialDevice, uint32_t speedNum) {
   int fd = open(serialDevice, O_RDWR | O_NOCTTY | O_SYNC);
   if (fd < 0) {
     errlog("error %d opening serial device %s: %s\n",
@@ -226,20 +221,20 @@ int openSerial(char *serialDevice, uint32_t speedNum) {
   return fd;
 }
 
-void closeSerial(int fd) {
+static void closeSerial(int fd) {
   loopControl(false);
   close(fd);
 }
 
-uint8_t request(int fd, uint8_t cmd, uint8_t addr, t_longframe **retFrame) {
+static /*@null@*/ t_longframe *request(int fd, uint8_t cmd, uint8_t addr) {
-  uint8_t retCode = SUCCESS;
+  errno = 0;
-
+  
   t_longframe *frame = (t_longframe*) malloc(sizeof(t_longframe));
   if (! frame) {
-    errlog("unable to allocate memory for frame\n");    
+    errlog("unable to allocate memory for frame\n");
-    return ERROR_OUT_OF_MEMORY__FRAME;
+    return NULL;
   }
-  frame->userdata = NULL;
+  memset(frame, 0, sizeof(t_longframe));
 
   uint8_t chksum = cmd + addr;
 
@@ -253,16 +248,20 @@ uint8_t request(int fd, uint8_t cmd, uint8_t addr, t_longframe **retFrame) {
   sendBuf[4] = 0x16;
   write(fd, sendBuf, 5);
 
-  while (1) {
+  while (true) {
-    int r;
+    int r = 0;
     if (ioctl(fd, TIOCSERGETLSR, &r) == -1) {
       errlog("error %d getting TIOCSERGETLSR for fd %d: %s\n",
              errno, fd, strerror(errno));
+      errno = ERROR_APP_SPECIFIC_ERROR_FLAG | ERROR_TX_REG_UNACCESSIBLE;
+      if (frame->userdata) {
+        free(frame->userdata);
+      }
       free(frame);
       frame = NULL;
-      return ERROR_TX_REG_UNACCESSIBLE;
+      return NULL;
     }
-    if (r & TIOCSER_TEMT) {
+    if ((r & TIOCSER_TEMT) != 0) {
       break;
     }
   }
@@ -275,6 +274,7 @@ uint8_t request(int fd, uint8_t cmd, uint8_t addr, t_longframe **retFrame) {
   t_state state = e_START1;
 
   while ((state != e_DONE) &&
+         (state != e_ERROR) &&
          (state != e_TIMEOUT)) {
      
     infolog("waiting for input ...\n");
@@ -295,21 +295,21 @@ uint8_t request(int fd, uint8_t cmd, uint8_t addr, t_longframe **retFrame) {
         state = e_LENGTH1;
       } else {
         errlog("invalid start1 symbol %02x\n", c);
-        retCode = ERROR_STATE_ENGINE__START1;
         state = e_ERROR;
       }
       break;
     case e_LENGTH1:
       if (c <= 3) {
         errlog("length to small %02x\n", c);
-        retCode = ERROR_STATE_ENGINE__LENGTH1;
         state = e_ERROR;
       } else {
         frame->length1 = c;
+        if (frame->userdata) {
+          free(frame->userdata);
+        }
         frame->userdata = (uint8_t*) malloc(frame->length1 - 3);
         if (! frame->userdata) {
           errlog("unable to allocate memory for userdata\n");
-          retCode = ERROR_OUT_OF_MEMORY__USERDATA;
           state = e_ERROR;
         }
         state = e_LENGTH2;
@@ -318,7 +318,6 @@ uint8_t request(int fd, uint8_t cmd, uint8_t addr, t_longframe **retFrame) {
     case e_LENGTH2:
       if (frame->length1 != c) {
         errlog("invalid length2 %02x vs. %02x\n", frame->length1, c);
-        retCode = ERROR_STATE_ENGINE__LENGTH2;
         state = e_ERROR;
       } else {
         frame->length2 = c;
@@ -331,7 +330,6 @@ uint8_t request(int fd, uint8_t cmd, uint8_t addr, t_longframe **retFrame) {
         state = e_C_FIELD;
       } else {
         errlog("invalid start2 symbol %02x\n", c);
-        retCode = ERROR_STATE_ENGINE__START2;
         state = e_ERROR;
       }
       break;
@@ -361,7 +359,6 @@ uint8_t request(int fd, uint8_t cmd, uint8_t addr, t_longframe **retFrame) {
     case e_CHKSUM:
       if (c != calculatedChksum) {
         errlog("invalid checksum %02x vs %02x\n", calculatedChksum, c);
-        retCode = ERROR_STATE_ENGINE__INVALID_CHKSUM;
         state = e_ERROR;
       } else {
         frame->chksum = c;
@@ -374,17 +371,11 @@ uint8_t request(int fd, uint8_t cmd, uint8_t addr, t_longframe **retFrame) {
         state = e_DONE;
       } else {
         errlog("invalid stop symbol %02x\n", c);
-        retCode = ERROR_STATE_ENGINE__STOP;
         state = e_ERROR;
       }
       break;
-    case e_ERROR:
-      ledRed(true);
-      infolog("already error, read the rest (now: %02x) until timeout\n", c);
-      break;
     default:
       errlog("illegal state %d\n", state);
-      retCode = ERROR_STATE_ENGINE__ILLEGAL_STATE;
       state = e_ERROR;
       break;
     }
@@ -392,13 +383,9 @@ uint8_t request(int fd, uint8_t cmd, uint8_t addr, t_longframe **retFrame) {
 
   if ((state == e_ERROR) || (state == e_TIMEOUT)) {
     if (state == e_ERROR) {
-      if (retCode == SUCCESS) {
+      errno = ERROR_TX_REG_UNACCESSIBLE | ERROR_STATE_ENGINE;
-        retCode = ERROR_STATE_ENGINE__UNKNOWN;
-      }
     } else if (state == e_TIMEOUT) {
-      if (retCode == SUCCESS) {
+      errno = ERROR_TX_REG_UNACCESSIBLE | ERROR_TIMEOUT;
-        retCode = ERROR_TIMEOUT;
-      }
     }
     if (frame->userdata) {
       free(frame->userdata);
@@ -408,17 +395,17 @@ uint8_t request(int fd, uint8_t cmd, uint8_t addr, t_longframe **retFrame) {
     frame = NULL;
   }
 
-  *retFrame = frame;
+  return frame;
-  return retCode;
 }
 
-void printFrame(bool hexOut, t_longframe *frame) {
+static void printFrame(bool hexOut, t_longframe *frame) {
   if (hexOut) {
     fprintf(stderr, "%02x %02x %02x %02x %02x %02x %02x\n",
             frame->start1, frame->length1, frame->length2, frame->start2,
             frame->c, frame->a, frame->ci);
-    for (uint8_t i = 0; i < (frame->length1 - 3); i++) {
+    uint8_t i = 0;
-      if (i && !(i % 16)) {
+    for (i = 0; i < (frame->length1 - 3); i++) {
+      if ((i != 0) && ((i % 16) == 0)) {
         fprintf(stderr, "\n");
       }
       fprintf(stderr, "%02x ", frame->userdata[i]);
@@ -430,7 +417,8 @@ void printFrame(bool hexOut, t_longframe *frame) {
     fprintf(stdout, "%c%c%c%c%c%c%c",
             frame->start1, frame->length1, frame->length2, frame->start2,
             frame->c, frame->a, frame->ci);
-    for (uint8_t i = 0; i < (frame->length1 - 3); i++) {
+    uint8_t i = 0;
+    for (i = 0; i < (frame->length1 - 3); i++) {
       fprintf(stdout, "%c", frame->userdata[i]);
     }
     fprintf(stdout, "%c%c", frame->chksum, frame->stop);
@@ -488,49 +476,41 @@ int main(int argc, char *argv[]) {
 
 
   infolog("opening device\n");
-  int fd = openSerial(DEFAULT_SERIAL_DEVICE, BAUDRATE);
+  int fd = openSerial(DEFAULT_SERIAL_DEVICE, 2400);
   if (fd == -1) {
     errlog("unable to open device, fatal error\n");
-    myExit(-1);
+    deinit();
+    exit(EXIT_FAILURE);
   }
 
 
-  while (1) {
+  while (true) {
-    if (lineMode) {
-      infolog("lineMode, waiting for input\n");
-      fread(&cmd, 1, 1, stdin);
-      fread(&addr, 1, 1, stdin);
-    }
-    if (lineMode && (cmd == LINEMODE_CMD_PREFIX)) {
-      if (addr == LINEMODE_CMD_TERMINATE) {
-        infolog("termination requested\n");
-        break;
-      }
-      if (addr == LINEMODE_CMD_LOOP_SHUTDOWN) {
-        infolog("loop shutdown requested\n");
-        loopControl(false);
-        continue;
-      }
-    }
-
     if (! loopActiveFlag) {
       errlog("loop is not active, enable it and delay\n");
       loopControl(true);
       msleep(2000);
     }
 
-    infolog("sending request %02x %02x\n", cmd, addr);
+    if (lineMode) {
-    t_longframe *frame = NULL;
+      infolog("lineMode, waiting for input\n");
-    uint8_t requestReturnCode = 0;
+      fread(&cmd, 1, 1, stdin);
-    if (loopActiveFlag) {
+      fread(&addr, 1, 1, stdin);
-      ledRed(false);
+    }
-      requestReturnCode = request(fd, cmd, addr, &frame);
+    if ((cmd == 0) && (addr == 0)) {
-    } else {
+      errlog("termination requested\n");
-      errlog("loop is currently inactive, no need to try\n");
+      break;
-      requestReturnCode = ERROR_LOOP_FAILURE;
     }
 
-    if (requestReturnCode == SUCCESS) {
+    infolog("sending request %02x %02x\n", cmd, addr);
+    t_longframe *frame = NULL;
+    if (loopActiveFlag) {
+      ledRed(false);
+      frame = request(fd, cmd, addr);
+    } else {
+      errlog("loop is currently inactive, no need to try\n");
+    }
+
+    if (frame) {
       infolog("received a valid frame\n");
       printFrame(hexOut, frame);
       free(frame->userdata);
@@ -540,11 +520,12 @@ int main(int argc, char *argv[]) {
     } else {
       ledRed(true);
       if (! loopActiveFlag) {
-        requestReturnCode = ERROR_LOOP_FAILURE;
+        errno = ERROR_APP_SPECIFIC_ERROR_FLAG | ERROR_LOOP_FAILURE;
       }
-      errlog("error %04x occured\n", requestReturnCode);
+      errlog("error %04x occured\n", errno);
       if (! hexOut) {
-        fprintf(stdout, "%c%c", requestReturnCode, 0);
+        uint8_t maskedError = (uint8_t)(errno & ~ERROR_APP_SPECIFIC_ERROR_FLAG);
+        fprintf(stdout, "%c%c", maskedError, 0);
         fflush(stdout);
       }
       if (! lineMode) {
@@ -560,7 +541,8 @@ int main(int argc, char *argv[]) {
   infolog("closing device\n");
   closeSerial(fd);
 
-  myExit(exitCode);
+  deinit();
+  return exitCode;
 }
 
 

src/mbusgw.h

@@ -16,26 +16,14 @@
 
 #define DEFAULT_SERIAL_DEVICE "/dev/ttyAMA0"
 
-#define BAUDRATE 2400
 
-#define LINEMODE_CMD_PREFIX 0
-#define LINEMODE_CMD_TERMINATE 0
-#define LINEMODE_CMD_LOOP_SHUTDOWN 1
-
-#define SUCCESS 0
 #define ERROR_TIMEOUT 1
-#define ERROR_LOOP_FAILURE 2
+#define ERROR_STATE_ENGINE 2
-#define ERROR_TX_REG_UNACCESSIBLE 3
+#define ERROR_LOOP_FAILURE 3
-#define ERROR_OUT_OF_MEMORY__FRAME 4
+#define ERROR_TX_REG_UNACCESSIBLE 4
-#define ERROR_OUT_OF_MEMORY__USERDATA 5
+
-#define ERROR_STATE_ENGINE__START1 10
+#define ERROR_APP_SPECIFIC_ERROR_FLAG 0x4000
-#define ERROR_STATE_ENGINE__LENGTH1 11
+
-#define ERROR_STATE_ENGINE__LENGTH2 12
-#define ERROR_STATE_ENGINE__START2 13
-#define ERROR_STATE_ENGINE__INVALID_CHKSUM 14
-#define ERROR_STATE_ENGINE__STOP 15
-#define ERROR_STATE_ENGINE__ILLEGAL_STATE 16
-#define ERROR_STATE_ENGINE__UNKNOWN 17
 
 
 typedef struct {
@@ -57,4 +45,4 @@ typedef struct {
 
 
 
-#endif
+#endif