Closes #98 - Fixes memory access for attribute masks and some other bugs associated with Valgrind errors

source/src/cip/cipcommon.c

@@ -27,8 +27,6 @@
 /* global public variables */
 EipUint8 g_message_data_reply_buffer[OPENER_MESSAGE_DATA_REPLY_BUFFER]; /**< Reply buffer */
 
-const EipUint16 kCipUintZero = 0; /**< Zero value for returning the UINT standard value */
-
 /* private functions*/
 int EncodeEPath(CipEpath *epath, EipUint8 **message);
 
@@ -312,12 +310,10 @@ void InsertAttribute(CipInstance *const instance,
 
       size_t index = CalculateIndex(attribute_number);
 
-      class->get_single_bit_mask[index] |= ( (cip_flags & 0x02) ? 1 : 0 )
-                                           << ( (attribute_number - 1) % 8 );
-      class->get_all_bit_mask[index] |= ( (cip_flags & 0x01) ? 1 : 0 )
-                                        << ( (attribute_number - 1) % 8 );
-      class->set_bit_mask[index] |= ( (cip_flags & 0x04) ? 1 : 0 )
-                                    << ( (attribute_number - 1) % 8 );
+      class->get_single_bit_mask[index] |= (cip_flags & kGetableSingle) ? 1 << (attribute_number) % 8 : 0 ;
+      class->get_all_bit_mask[index] |= (cip_flags & kGetableAll) ? 1 << (attribute_number) % 8 : 0 ;
+      class->set_bit_mask[index] |= ( (cip_flags & kSetable) ? 1 : 0 )
+                                    << ( (attribute_number) % 8 );
 
       return;
     }
@@ -395,20 +391,19 @@ EipStatus GetAttributeSingle(
 
   EipUint16 attribute_number = message_router_request->request_path
                                .attribute_number;
-  uint8_t get_bit_mask;
-
-  if (kGetAttributeAll == message_router_request->service) {
-    get_bit_mask = (instance->cip_class->get_all_bit_mask[CalculateIndex(
-                                                            attribute_number)]);
-    message_router_response->general_status = kCipErrorSuccess;
-  } else {
-    get_bit_mask = (instance->cip_class->get_single_bit_mask[CalculateIndex(
-                                                               attribute_number)
-                    ]);
-  }
 
-  if ( (attribute != 0) && (attribute->data != 0) ) {
-    if ( get_bit_mask & ( 1 << ( (attribute_number - 1) % 8 ) ) ) {
+  if ( (NULL != attribute) && (NULL != attribute->data) ) {
+	  uint8_t get_bit_mask = 0;
+	  if (kGetAttributeAll == message_router_request->service) {
+	      get_bit_mask = (instance->cip_class->get_all_bit_mask[CalculateIndex(
+	                                                              attribute_number)]);
+	      message_router_response->general_status = kCipErrorSuccess;
+	    } else {
+	      get_bit_mask = (instance->cip_class->get_single_bit_mask[CalculateIndex(
+	                                                                 attribute_number)
+	                      ]);
+	    }
+    if (0 != (get_bit_mask & ( 1 << ( attribute_number % 8 ) ) ) ) {
       OPENER_TRACE_INFO("getAttribute %d\n",
                         message_router_request->request_path.attribute_number); /* create a reply message containing the data*/
 
@@ -424,6 +419,7 @@ EipStatus GetAttributeSingle(
         BeforeAssemblyDataSend(instance);
       }
 
+      OPENER_ASSERT(NULL != attribute);
       message_router_response->data_length = EncodeData(attribute->type,
                                                         attribute->data,
                                                         &message);
@@ -704,7 +700,7 @@ EipStatus GetAttributeAll(CipInstance *instance,
           if ( attrNum < 32
                && ( (instance->cip_class->get_all_bit_mask[CalculateIndex(
                                                              attrNum)])
-                    & ( 1 << ( (attrNum - 1) % 8 ) ) ) ) /* only return attributes that are flagged as being part of GetAttributeALl */
+                    & ( 1 << ( attrNum % 8 ) ) ) ) /* only return attributes that are flagged as being part of GetAttributeALl */
           {
             message_router_request->request_path.attribute_number = attrNum;
             if ( kEipStatusOkSend
@@ -858,14 +854,15 @@ int DecodePaddedEPath(CipEpath *epath, const EipUint8 **message) {
 }
 
 void AllocateAttributeMasks(CipClass *target_class) {
-  size_t size = (target_class->highest_attribute_number + 8 - 1) / 8;
+  size_t size = 1 + CalculateIndex(target_class->highest_attribute_number);
+  OPENER_TRACE_INFO(">>> Allocate memory for %s %lu bytes times 3 for masks\n", target_class->class_name, size);
   target_class->get_single_bit_mask = CipCalloc( size, sizeof(uint8_t) );
   target_class->set_bit_mask = CipCalloc( size, sizeof(uint8_t) );
   target_class->get_all_bit_mask = CipCalloc( size, sizeof(uint8_t) );
 }
 
 size_t CalculateIndex(EipUint16 attribute_number) {
-  size_t index = (attribute_number + 8 - 1) / 8;
+  size_t index = attribute_number / 8;
   return index;
 }
 

source/src/cip/cipcommon.h

@@ -21,6 +21,8 @@
  */
 extern EipUint8 g_message_data_reply_buffer[];
 
+static const EipUint16 kCipUintZero = 0; /**< Zero value for returning the UINT standard value */
+
 /** @brief Check if requested service present in class/instance and call appropriate service.
  *
  * @param class class receiving the message

source/src/cip/cipconnectionmanager.c

@@ -230,7 +230,6 @@ EipUint32 GetConnectionId(void) {
 }
 
 void InitializeConnectionManager(CipClass *class) {
-  const EipUint16 kCipUintZero = 0;
 
   CipClass *meta_class = class->class_instance.cip_class;
 

source/src/cip/cipidentity.c

@@ -119,27 +119,27 @@ static EipStatus Reset(CipInstance *instance,
 }
 
 void InitializeCipIdentiy(CipClass *class) {
-  const EipUint16 kCipUintZero = 0;
+
 
   CipClass *meta_class = class->class_instance.cip_class;
 
   InsertAttribute( (CipInstance *) class, 1, kCipUint,
                    (void *) &class->revision,
-                   kGetableSingleAndAll ); /* revision */
+				   kGetableSingleAndAll ); /* revision */
   InsertAttribute( (CipInstance *) class, 2, kCipUint,
                    (void *) &class->number_of_instances, kGetableSingleAndAll ); /*  largest instance number */
   InsertAttribute( (CipInstance *) class, 3, kCipUint,
-                   (void *) &class->number_of_instances, kGetableSingle ); /* number of instances currently existing*/
+                   (void *) &class->number_of_instances, kGetAttributeSingle ); /* number of instances currently existing*/
   InsertAttribute( (CipInstance *) class, 4, kCipUint, (void *) &kCipUintZero,
-                   kNotSetOrGetable ); /* optional attribute list - default = 0 */
+		  kNotSetOrGetable ); /* optional attribute list - default = 0 */
   InsertAttribute( (CipInstance *) class, 5, kCipUint, (void *) &kCipUintZero,
-                   kNotSetOrGetable ); /* optional service list - default = 0 */
+		  kNotSetOrGetable ); /* optional service list - default = 0 */
   InsertAttribute( (CipInstance *) class, 6, kCipUint,
                    (void *) &meta_class->highest_attribute_number,
-                   kGetableSingleAndAll ); /* max class attribute number*/
+				   kGetableSingleAndAll ); /* max class attribute number*/
   InsertAttribute( (CipInstance *) class, 7, kCipUint,
                    (void *) &class->highest_attribute_number,
-                   kGetableSingleAndAll ); /* max instance attribute number*/
+				   kGetableSingleAndAll ); /* max instance attribute number*/
 
 }
 
@@ -149,7 +149,7 @@ EipStatus CipIdentityInit() {
                                    7, /* # highest class attribute number*/
                                    2, /* # of class services*/
                                    7, /* # of instance attributes*/
-                                   19, /* # highest instance attribute number*/
+                                   7, /* # highest instance attribute number*/
                                    3, /* # of instance services*/
                                    1, /* # of instances*/
                                    "identity", /* # class name (for debug)*/

source/src/cip/ciptcpipinterface.c

@@ -139,11 +139,11 @@ EipStatus SetAttributeSingleTcp(
   (void) instance; /*Suppress compiler warning */
   EipUint16 attribute_number = message_router_request->request_path
                                .attribute_number;
-  uint8_t set_bit_mask = (instance->cip_class->set_bit_mask[CalculateIndex(
-                                                              attribute_number)]);
 
   if (NULL != attribute) {
-    if ( set_bit_mask & ( 1 << ( (attribute_number - 1) % 8 ) ) ) {
+  uint8_t set_bit_mask = (instance->cip_class->set_bit_mask[CalculateIndex(
+                                                              attribute_number)]);
+    if ( set_bit_mask & ( 1 << ( (attribute_number) % 8 ) ) ) {
       switch (attribute_number) {
         case 3: {
           CipUint configuration_control_recieved = GetDintFromMessage(
@@ -292,21 +292,15 @@ EipStatus GetAttributeSingleTcpIpInterface(
 
   EipUint16 attribute_number = message_router_request->request_path
                                .attribute_number;
-  uint8_t get_bit_mask;
+  uint8_t get_bit_mask = 0;
 
-  if (kGetAttributeAll == message_router_request->service) {
-    get_bit_mask = (instance->cip_class->get_all_bit_mask[CalculateIndex(
-                                                            attribute_number)]);
-    message_router_response->general_status = kCipErrorSuccess;
-  } else {
-    get_bit_mask = (instance->cip_class->get_single_bit_mask[CalculateIndex(
-                                                               attribute_number)
-                    ]);
-  }
+  message_router_response->general_status = kCipErrorAttributeNotSupported;
 
-  if ( get_bit_mask & ( 1 << ( (attribute_number - 1) % 8 ) ) ) {
-    if (9 == message_router_request->request_path.attribute_number) { /* attribute 9 can not be easily handled with the default mechanism therefore we will do it by hand */
+    if (9 == message_router_request->request_path.attribute_number ) { /* attribute 9 can not be easily handled with the default mechanism therefore we will do it by hand */
 
+    	if (0 == (get_bit_mask & ( 1 << ( attribute_number  % 8 ) )) ) {
+    		return kEipStatusOkSend;
+    	}
       message_router_response->general_status = kCipErrorSuccess;
 
       message_router_response->data_length += EncodeData(
@@ -329,12 +323,26 @@ EipStatus GetAttributeSingleTcpIpInterface(
       CipAttributeStruct *attribute = GetCipAttribute(instance,
                                                       attribute_number);
 
-      if ( (attribute != 0) && (attribute->data != 0) ) {
+      if ( (NULL != attribute) && ( NULL != attribute->data)) {
 
         OPENER_TRACE_INFO(
           "getAttribute %d\n",
           message_router_request->request_path.attribute_number);   /* create a reply message containing the data*/
 
+        if (kGetAttributeAll == message_router_request->service) {
+            get_bit_mask = (instance->cip_class->get_all_bit_mask[CalculateIndex(
+                                                                    attribute_number)]);
+            message_router_response->general_status = kCipErrorSuccess;
+          } else {
+            get_bit_mask = (instance->cip_class->get_single_bit_mask[CalculateIndex(
+                                                                       attribute_number)
+                            ]);
+          }
+
+        if(0 == (get_bit_mask &  (1 << ( (attribute_number ) % 8 ) ) ) ) {
+        	return kEipStatusOkSend;
+        }
+
         /*TODO think if it is better to put this code in an own
          * getAssemblyAttributeSingle functions which will call get attribute
          * single.
@@ -353,28 +361,6 @@ EipStatus GetAttributeSingleTcpIpInterface(
         message_router_response->general_status = kCipErrorSuccess;
       }
     }
-  } else {
-    message_router_response->general_status = kCipErrorAttributeNotSupported;
-  }
-
-  //TEST
-  CipClass *class = instance->cip_class;
-  for (int i = 1; i <= class->highest_attribute_number; i++) {
-    size_t index = (i + 8 - 1) / 8;
-    OPENER_TRACE_INFO(
-      "Attribute %d: %d / %d / %d\n",
-      i,
-      ( ( (class->get_all_bit_mask)[index] ) & ( 1 << ( (i - 1) % 8 ) ) ) ? 1 : 0,
-      ( ( (class->get_single_bit_mask)[index] ) & ( 1 << ( (i - 1) % 8 ) ) ) ? 1 : 0,
-      ( ( (class->set_bit_mask)[index] ) & ( 1 << ( (i - 1) % 8 ) ) ) ? 1 : 0);
-  }
-  OPENER_TRACE_INFO("GetAll, byte1: %d, byte2: %d\n",
-                    (class->get_all_bit_mask)[1], (class->get_all_bit_mask)[2]);
-  OPENER_TRACE_INFO("GetSingle, byte1: %d, byte2: %d\n",
-                    (class->get_single_bit_mask)[1],
-                    (class->get_single_bit_mask)[2]);
-  OPENER_TRACE_INFO("Set, byte1: %d, byte2: %d\n", (class->set_bit_mask)[1],
-                    (class->set_bit_mask)[2]);
 
   return kEipStatusOkSend;
 }