Merge pull request #302 from EIPStackGroup/SC_Issue_ConfFile

Tries to fix SonarClouds problems

source/src/ports/nvdata/conffile.c

@@ -34,19 +34,19 @@
 #define CFG_BASE  "nvdata/"
 
 #if ENABLE_VERBOSE != 0
-#define VERBOSE(pFile, pFmt, ...)   do { fprintf(pFile, pFmt, ##__VA_ARGS__); } while (0)
+#define VERBOSE(pFile, pFmt, ...)   do { fprintf(pFile, pFmt, ## __VA_ARGS__); \
+} while (0)
 #else
 #define VERBOSE(pFile, pFmt, ...)
 #endif
 
 
 /** @brief Portable wrapper for mkdir(). Internally used by RecMkdir()
-  *
-  * @param[in] path the full path of the directory to create
-  * @return zero on success, otherwise -1 and errno set
-  */
-static inline int Mkdir(const char *path)
-{
+ *
+ * @param[in] path the full path of the directory to create
+ * @return zero on success, otherwise -1 and errno set
+ */
+static inline int Mkdir(const char *path) {
 #ifdef _WIN32
   return _mkdir(path);
 #else
@@ -56,8 +56,7 @@ static inline int Mkdir(const char *path)
 }
 
 
-static void RecMkdir(char * const p_path)
-{
+static void RecMkdir(char *const p_path) {
   char *sep = strrchr(p_path, '/' );
   if(sep && p_path != sep) {  /* "p_path != sep" avoids mkdir("/")! */
     *sep = '\0';
@@ -74,8 +73,8 @@ static void RecMkdir(char * const p_path)
 }
 
 
-static FILE *FopenMkdir(char *p_path, char *mode)
-{
+static FILE *FopenMkdir(char *p_path,
+                        char *mode) {
   char *sep = strrchr(p_path, '/' );
   /* In write mode create missing directories. */
   if(sep && 'w' == *mode) {
@@ -98,23 +97,16 @@ static FILE *FopenMkdir(char *p_path, char *mode)
  *  This function open a configuration file, possibly for write operation,
  *  in the NV data storage directory.
  */
-int ConfFileOpen(bool write, const char *p_name, FILE **p_filep)
-{
-  char  path_buf[64];
-  int   rc;
+FILE *ConfFileOpen(const bool write,
+                   const char *const p_name) {
+  char path_buf[64];
+  int rc;
 
   rc = snprintf(path_buf, sizeof path_buf, "%s%s", CFG_BASE, p_name);
   if (rc > 0) {
-    FILE *filep = FopenMkdir(path_buf, write ? "w" : "r");
-    if (filep) {
-      rc = 0;
-      *p_filep  = filep;
-    }
-    else {
-      rc = EOF;
-    }
+    return FopenMkdir(path_buf, write ? "w" : "r");
   }
-  return rc;
+  return NULL;
 }
 
 /** @brief Close the configuration file associated with the FILE* given
@@ -125,9 +117,11 @@ int ConfFileOpen(bool write, const char *p_name, FILE **p_filep)
  *  Closes the configuration file associated to p_filep. No data
  *  synchronization to disk yet.
  */
-int ConfFileClose(FILE **p_filep)
-{
-  int rc = fclose(*p_filep);
+EipStatus ConfFileClose(FILE **p_filep) {
+  EipStatus eip_status = kEipStatusOk;
+  if(0 != fclose(*p_filep) ) {
+    eip_status = kEipStatusError;
+  }
   *p_filep = NULL;
-  return rc;
+  return eip_status;
 }

source/src/ports/nvdata/conffile.h

@@ -14,8 +14,11 @@
 #include <stdbool.h>
 #include <stdio.h>
 
-int ConfFileOpen(bool write, const char *p_name, FILE **p_filep);
+#include "typedefs.h"
 
-int ConfFileClose(FILE **p_filep);
+FILE *ConfFileOpen(const bool write,
+                   const char *const p_name);
+
+EipStatus ConfFileClose(FILE **p_filep);
 
 #endif  /* _CONFFILE_H_ */

source/src/ports/nvdata/nvdata.c

@@ -34,43 +34,38 @@
  *  and return (-1) on failure and (0) on success.
  */
 EipStatus NvdataLoad(void) {
-  EipStatus status = kEipStatusOk;
-  int       rc;
-
   /* Load NV data for QoS object instance */
-  rc = NvQosLoad(&g_qos);
-  status |= rc;
-  if (0 != rc) {
-    rc = NvQosStore(&g_qos);
-    status |= rc;
+  EipStatus eip_status = NvQosLoad(&g_qos);
+  if (kEipStatusError != eip_status) {
+    eip_status =
+      (kEipStatusError == NvQosStore(&g_qos) ) ? kEipStatusError : eip_status;
   }
 
-  return status;
+  return eip_status;
 }
 
 /** A PostSetCallback for QoS class to store NV attributes
-*
-* @param  instance  pointer to instance of QoS class
-* @param  attribute pointer to attribute structure
-* @param  service   the CIP service code of current request
-*
-* This function implements the PostSetCallback for the QoS class. The
-* purpose of this function is to save the NV attributes of the QoS
-* class instance to external storage.
-*
-* This application specific implementation chose to save all attributes
-* at once using a single NvQosStore() call.
-*/
+ *
+ * @param  instance  pointer to instance of QoS class
+ * @param  attribute pointer to attribute structure
+ * @param  service   the CIP service code of current request
+ *
+ * This function implements the PostSetCallback for the QoS class. The
+ * purpose of this function is to save the NV attributes of the QoS
+ * class instance to external storage.
+ *
+ * This application specific implementation chose to save all attributes
+ * at once using a single NvQosStore() call.
+ */
 EipStatus NvQosSetCallback
 (
   CipInstance *const instance,
   CipAttributeStruct *const attribute,
   CipByte service
-)
-{
+) {
   EipStatus status = kEipStatusOk;
 
-  if (0 != (kNvDataFunc & attribute->attribute_flags)) {
+  if (0 != (kNvDataFunc & attribute->attribute_flags) ) {
     OPENER_TRACE_INFO("NV data update: %s, i %" PRIu32 ", a %" PRIu16 "\n",
                       instance->cip_class->class_name,
                       instance->instance_number,
@@ -81,36 +76,35 @@ EipStatus NvQosSetCallback
 }
 
 /** A PostSetCallback for TCP/IP class to store NV attributes
-*
-* @param  instance  pointer to instance of TCP/IP class
-* @param  attribute pointer to attribute structure
-* @param  service   the CIP service code of current request
-*
-* This function implements the PostSetCallback for the TCP/IP class. The
-* purpose of this function is to save the NV attributes of the TCP/IP
-* class instance to external storage.
-*
-* This application specific implementation chose to save all attributes
-* at once using a single NvTcpipStore() call.
-*/
+ *
+ * @param  instance  pointer to instance of TCP/IP class
+ * @param  attribute pointer to attribute structure
+ * @param  service   the CIP service code of current request
+ *
+ * This function implements the PostSetCallback for the TCP/IP class. The
+ * purpose of this function is to save the NV attributes of the TCP/IP
+ * class instance to external storage.
+ *
+ * This application specific implementation chose to save all attributes
+ * at once using a single NvTcpipStore() call.
+ */
 EipStatus NvTcpipSetCallback
 (
-    CipInstance *const instance,
-    CipAttributeStruct *const attribute,
-    CipByte service
-)
-{
-    EipStatus status = kEipStatusOk;
+  CipInstance *const instance,
+  CipAttributeStruct *const attribute,
+  CipByte service
+) {
+  EipStatus status = kEipStatusOk;
 
-    if (0 != (kNvDataFunc & attribute->attribute_flags)) {
-      /* Workaround: Update only if service is not flagged. */
-      if (0 == (0x80 & service)) {
-        OPENER_TRACE_INFO("NV data update: %s, i %" PRIu32 ", a %" PRIu16 "\n",
-                          instance->cip_class->class_name,
-                          instance->instance_number,
-                          attribute->attribute_number);
-        status = NvTcpipStore(&g_tcpip);
-      }
+  if (0 != (kNvDataFunc & attribute->attribute_flags) ) {
+    /* Workaround: Update only if service is not flagged. */
+    if (0 == (0x80 & service) ) {
+      OPENER_TRACE_INFO("NV data update: %s, i %" PRIu32 ", a %" PRIu16 "\n",
+                        instance->cip_class->class_name,
+                        instance->instance_number,
+                        attribute->attribute_number);
+      status = NvTcpipStore(&g_tcpip);
     }
-    return status;
+  }
+  return status;
 }

source/src/ports/nvdata/nvqos.c

@@ -29,9 +29,8 @@
  *  @return       0: success; -1: failure
  */
 int NvQosLoad(CipQosObject *p_qos) {
-  FILE  *p_file;
   int rd_cnt = 0;
-  int rc;
+  EipStatus eip_status = kEipStatusError;
 
   CipUsint dscp_urgent = 0;
   CipUsint dscp_scheduled = 0;
@@ -39,8 +38,8 @@ int NvQosLoad(CipQosObject *p_qos) {
   CipUsint dscp_low = 0;
   CipUsint dscp_explicit = 0;
 
-  rc = ConfFileOpen(false, QOS_CFG_NAME, &p_file);
-  if (0 == rc) {
+  FILE  *p_file = ConfFileOpen(false, QOS_CFG_NAME);
+  if (NULL != p_file) {
     /* Read input data */
     rd_cnt = fscanf(p_file,
                     " %" SCNu8 ", %" SCNu8 ", %" SCNu8 ", %" SCNu8 ", %" SCNu8 "\n",
@@ -51,9 +50,9 @@ int NvQosLoad(CipQosObject *p_qos) {
                     &dscp_explicit);
 
     /* Need to try to close all stuff in any case. */
-    rc = ConfFileClose(&p_file);
+    eip_status = ConfFileClose(&p_file);
   }
-  if (0 == rc) {
+  if (kEipStatusOk == eip_status) {
     /* If all data were read copy them to the global QoS object. */
     if (5 == rd_cnt) {
       p_qos->dscp.urgent = dscp_urgent;
@@ -61,9 +60,11 @@ int NvQosLoad(CipQosObject *p_qos) {
       p_qos->dscp.high = dscp_high;
       p_qos->dscp.low = dscp_low;
       p_qos->dscp.explicit = dscp_explicit;
+    } else {
+      eip_status = kEipStatusError;
     }
   }
-  return rc;
+  return eip_status;
 }
 
 /** @brief Store NV data of the QoS object to file
@@ -71,26 +72,26 @@ int NvQosLoad(CipQosObject *p_qos) {
  *  @param  p_qos pointer to the QoS object's data structure
  *  @return       0: success; -1: failure
  */
-int NvQosStore(const CipQosObject *p_qos) {
-  FILE  *p_file;
-  int rc;
-
-  rc = ConfFileOpen(true, QOS_CFG_NAME, &p_file);
-  if (rc >= 0) {
+EipStatus NvQosStore(const CipQosObject *p_qos) {
+  FILE  *p_file = ConfFileOpen(true, QOS_CFG_NAME);
+  EipStatus eip_status = kEipStatusOk;
+  if (NULL != p_file) {
     /* Print output data */
-    rc = fprintf(p_file,
-                 " %" PRIu8 ", %" PRIu8 ", %" PRIu8 ", %" PRIu8 ", %" PRIu8 "\n",
-                 p_qos->dscp.urgent,
-                 p_qos->dscp.scheduled,
-                 p_qos->dscp.high,
-                 p_qos->dscp.low,
-                 p_qos->dscp.explicit);
-    if (rc > 0) {
-      rc = 0;
+    if (0 >= fprintf(p_file,
+                     " %" PRIu8 ", %" PRIu8 ", %" PRIu8 ", %" PRIu8 ", %" PRIu8
+                     "\n",
+                     p_qos->dscp.urgent,
+                     p_qos->dscp.scheduled,
+                     p_qos->dscp.high,
+                     p_qos->dscp.low,
+                     p_qos->dscp.explicit) ) {
+      eip_status = kEipStatusError;
     }
 
     /* Need to try to close all stuff in any case. */
-    rc |= ConfFileClose(&p_file);
+    eip_status =
+      (kEipStatusError ==
+       ConfFileClose(&p_file) ) ? kEipStatusError : eip_status;
   }
-  return rc;
+  return eip_status;
 }

source/src/ports/nvdata/nvtcpip.c

@@ -28,29 +28,28 @@
  *  @param  p_qos pointer to the TCP/IP object's data structure
  *  @return       0: success; -1: failure
  */
-int NvTcpipLoad(CipTcpIpObject *p_tcp_ip)
-{
-  CipTcpIpObject  tcpip;
-  FILE  *p_file;
-  int   rc;
+int NvTcpipLoad(CipTcpIpObject *p_tcp_ip) {
+  CipTcpIpObject tcpip = {0};
+  EipStatus eip_status = kEipStatusOk;
 
-  memset(&tcpip, 0, sizeof tcpip);
-  rc = ConfFileOpen(false, TCPIP_CFG_NAME, &p_file);
-  if (0 == rc) {
+  FILE *p_file = ConfFileOpen(false, TCPIP_CFG_NAME);
+  if (NULL != p_file) {
     /* Read input data */
-    OPENER_TRACE_ERR("ERROR: Loading of TCP/IP object's NV data not implemented yet\n");
+    OPENER_TRACE_ERR(
+      "ERROR: Loading of TCP/IP object's NV data not implemented yet\n");
     /* TODO: Implement load */
-    rc = kEipStatusError;
-
-    /* Need to try to close all stuff in any case. */
-    rc = ConfFileClose(&p_file);
-  }
-  if (0 == rc) {
+    eip_status = kEipStatusError;
     /* If all NV attributes were read copy them attribute by attribute
      * to the caller's TCP/IP object. */
     /* TODO: copy all NV attributes */
+
+    /* Need to try to close all stuff in any case. */
+    eip_status =
+      (kEipStatusError ==
+       ConfFileClose(&p_file) ) ? kEipStatusError : eip_status;
   }
-  return rc;
+
+  return eip_status;
 }
 
 /** @brief Store NV data of the TCP/IP object to file
@@ -58,20 +57,19 @@ int NvTcpipLoad(CipTcpIpObject *p_tcp_ip)
  *  @param  p_qos pointer to the TCP/IP object's data structure
  *  @return       0: success; -1: failure
  */
-int NvTcpipStore(const CipTcpIpObject *p_tcp_ip)
-{
-  FILE  *p_file;
-  int   rc;
-
-  rc = ConfFileOpen(true, TCPIP_CFG_NAME, &p_file);
-  if (rc >= 0) {
+EipStatus NvTcpipStore(const CipTcpIpObject *p_tcp_ip) {
+  FILE *p_file = ConfFileOpen(true, TCPIP_CFG_NAME);
+  if (NULL != p_file) {
     /* Print output data */
-    OPENER_TRACE_ERR("ERROR: Storing of TCP/IP object's NV data not implemented yet\n");
+    OPENER_TRACE_ERR(
+      "ERROR: Storing of TCP/IP object's NV data not implemented yet\n");
     /* TODO: Implement store */
-    rc = kEipStatusError;
+    EipStatus eip_status = kEipStatusError;
 
     /* Need to try to close all stuff in any case. */
-    rc |= ConfFileClose(&p_file);
+    return (kEipStatusError ==
+            ConfFileClose(&p_file) ) ? kEipStatusError : eip_status;
+  } else {
+    return kEipStatusError; /* File could not be openend*/
   }
-  return rc;
 }