From f59f486bb2c4713dc2ba5fe6064fde55f5630a97 Mon Sep 17 00:00:00 2001 From: Jason Valenzuela Date: Fri, 4 Mar 2022 23:15:16 -0500 Subject: [PATCH] Create custom data type for object instance numbers. This is intended to help ensure uniform data type for instance numbers, resolving type conversion warnings arising from assignment between instance numbers of different widths, e.g., Visual Studio warning C4244. --- source/src/cip/cipassembly.c | 2 +- source/src/cip/cipcommon.c | 14 ++++---- source/src/cip/cipconnectionmanager.c | 34 +++++++++++++------ source/src/cip/cipconnectionmanager.h | 2 +- source/src/cip/cipepath.h | 2 +- source/src/cip/cipethernetlink.c | 6 ++-- source/src/cip/cipmessagerouter.c | 2 +- source/src/cip/ciptypes.c | 4 +++ source/src/cip/ciptypes.h | 14 ++++++-- source/src/opener_api.h | 10 +++--- .../MINGW/sample_application/ethlinkcbs.c | 4 +-- .../POSIX/sample_application/ethlinkcbs.c | 4 +-- .../WIN32/sample_application/ethlinkcbs.c | 4 +-- source/src/typedefs.h | 11 ++++++ 14 files changed, 75 insertions(+), 38 deletions(-) diff --git a/source/src/cip/cipassembly.c b/source/src/cip/cipassembly.c index 0adea5a425..413513eb41 100644 --- a/source/src/cip/cipassembly.c +++ b/source/src/cip/cipassembly.c @@ -95,7 +95,7 @@ void ShutdownAssemblies(void) { } } -CipInstance *CreateAssemblyObject(const EipUint32 instance_id, +CipInstance *CreateAssemblyObject(const CipInstanceNum instance_id, EipByte *const data, const EipUint16 data_length) { CipClass *assembly_class = GetCipClass(kCipAssemblyClassCode); diff --git a/source/src/cip/cipcommon.c b/source/src/cip/cipcommon.c index 51df75fccd..62f9b4d189 100644 --- a/source/src/cip/cipcommon.c +++ b/source/src/cip/cipcommon.c @@ -105,7 +105,7 @@ EipStatus NotifyClass(const CipClass *RESTRICT const cip_class, const int encapsulation_session) { /* find the instance: if instNr==0, the class is addressed, else find the instance */ - EipUint16 instance_number = + CipInstanceNum instance_number = message_router_request->request_path.instance_number; /* get the instance number */ CipInstance *instance = GetCipInstance(cip_class, instance_number); /* look up the instance (note that if inst==0 this will be the class itself) */ if(instance) /* if instance is found */ @@ -165,10 +165,10 @@ CipUint GetMaxInstanceNumber(CipClass *RESTRICT const cip_class) { } CipInstance *AddCipInstances(CipClass *RESTRICT const cip_class, - const int number_of_instances) { + const CipInstanceNum number_of_instances) { CipInstance **next_instance = NULL; CipInstance *first_instance = NULL; /* Initialize to error result */ - EipUint32 instance_number = 1; /* the first instance is number 1 */ + CipInstanceNum instance_number = 1; /* the first instance is number 1 */ int new_instances = 0; OPENER_TRACE_INFO("adding %d instances to class %s\n", @@ -248,7 +248,7 @@ CipInstance *AddCipInstances(CipClass *RESTRICT const cip_class, } CipInstance *AddCipInstance(CipClass *RESTRICT const cip_class, - const EipUint32 instance_id) { + const CipInstanceNum instance_id) { CipInstance *instance = GetCipInstance(cip_class, instance_id); if(NULL == instance) { /*we have no instance with given id*/ @@ -268,7 +268,7 @@ CipClass *CreateCipClass(const CipUdint class_code, const int number_of_instance_attributes, const EipUint32 highest_instance_attribute_number, const int number_of_instance_services, - const int number_of_instances, + const CipInstanceNum number_of_instances, const char *const name, const EipUint16 revision, InitializeCipClass initializer) { @@ -331,7 +331,7 @@ CipClass *CreateCipClass(const CipUdint class_code, cip_class->class_instance.cip_class = meta_class; /* the class's class is the metaclass (like SmallTalk)*/ cip_class->class_instance.next = 0; /* the next link will always be zero, since there is only one instance of any particular class object */ - meta_class->class_instance.instance_number = 0xffffffff; /*the metaclass object does not really have a valid instance number*/ + meta_class->class_instance.instance_number = 0xffff; /*the metaclass object does not really have a valid instance number*/ meta_class->class_instance.attributes = NULL;/* the metaclass has no attributes*/ meta_class->class_instance.cip_class = NULL; /* the metaclass has no class*/ meta_class->class_instance.next = NULL; /* the next link will always be zero, since there is only one instance of any particular metaclass object*/ @@ -1355,7 +1355,7 @@ void EncodeEPath(const CipEpath *const epath, if(0 < length) { if(epath->instance_number < 256) { AddSintToMessage(0x24, message); /*8Bit Instance Id */ - AddSintToMessage(epath->instance_number, message); + AddSintToMessage( (EipUint8)epath->instance_number, message ); length -= 1; } else { AddSintToMessage(0x25, message); /*16Bit Instance Id */ diff --git a/source/src/cip/cipconnectionmanager.c b/source/src/cip/cipconnectionmanager.c index 845612bff0..e5609bcc1f 100644 --- a/source/src/cip/cipconnectionmanager.c +++ b/source/src/cip/cipconnectionmanager.c @@ -1342,7 +1342,7 @@ EipUint8 ParseConnectionPath(CipConnectionObject *connection_object, CipClass *class = NULL; CipDword class_id = 0x0; - CipDword instance_id = 0x0; + CipInstanceNum instance_id = 0x0; /* with 256 we mark that we haven't got a PIT segment */ ConnectionObjectSetProductionInhibitTime(connection_object, 256); @@ -1473,16 +1473,18 @@ EipUint8 ParseConnectionPath(CipConnectionObject *connection_object, if(kSegmentTypeLogicalSegment == GetPathSegmentType(message) && kLogicalSegmentLogicalTypeInstanceId == GetPathLogicalSegmentLogicalType(message) ) { /* store the configuration ID for later checking in the application connection types */ - instance_id = CipEpathGetLogicalValue(&message); + const CipDword temp_id = CipEpathGetLogicalValue(&message); OPENER_TRACE_INFO("Configuration instance id %" PRId32 "\n", - instance_id); - if(NULL == GetCipInstance(class, instance_id) ) { + temp_id); + if( (temp_id > kCipInstanceNumMax) || + ( NULL == GetCipInstance(class, (CipInstanceNum)temp_id) ) ) { /*according to the test tool we should respond with this extended error code */ *extended_error = kConnectionManagerExtendedStatusCodeErrorInvalidSegmentTypeInPath; return kCipErrorConnectionFailure; } + instance_id = (CipInstanceNum)temp_id; /* 1 or 2 16Bit words for the configuration instance part of the path */ remaining_path -= (instance_id > 0xFF) ? 2 : 1; //TODO: 32 bit case missing } else { @@ -1570,22 +1572,32 @@ EipUint8 ParseConnectionPath(CipConnectionObject *connection_object, || kLogicalSegmentLogicalTypeConnectionPoint == GetPathLogicalSegmentLogicalType(message) ) ) /* Connection Point interpreted as InstanceNr -> only in Assembly Objects */ { /* Attribute Id or Connection Point */ - CipDword attribute_id = CipEpathGetLogicalValue(&message); - CipConnectionPathEpath path = - { .class_id = class_id, .instance_id = attribute_id, - .attribute_id_or_connection_point = 0 }; + + /* Validate encoded instance number. */ + const CipDword temp_instance_id = CipEpathGetLogicalValue(&message); + if (temp_instance_id > kCipInstanceNumMax) { + *extended_error = + kConnectionManagerExtendedStatusCodeErrorInvalidSegmentTypeInPath; + return kCipErrorConnectionFailure; + } + instance_id = (CipInstanceNum)temp_instance_id; + + CipConnectionPathEpath path; + path.class_id = class_id; + path.instance_id = instance_id; + path.attribute_id_or_connection_point = 0; memcpy(paths_to_encode[i], &path, sizeof(connection_object->produced_path) ); OPENER_TRACE_INFO( "connection point %" PRIu32 "\n", attribute_id); - if(NULL == GetCipInstance(class, attribute_id) ) { /* Old code - Probably here the attribute ID marks the instance for the assembly object */ + if( NULL == GetCipInstance(class, instance_id) ) { *extended_error = kConnectionManagerExtendedStatusCodeInconsistentApplicationPathCombo; return kCipErrorConnectionFailure; } /* 1 or 2 16Bit word for the connection point part of the path */ - remaining_path -= (attribute_id > 0xFF) ? 2 : 1; + remaining_path -= (instance_id > 0xFF) ? 2 : 1; } else { *extended_error = kConnectionManagerExtendedStatusCodeErrorInvalidSegmentTypeInPath; @@ -1695,7 +1707,7 @@ void RemoveFromActiveConnections(CipConnectionObject *const connection_object) { } OPENER_TRACE_ERR("Connection not found in active connection list\n"); } -EipBool8 IsConnectedOutputAssembly(const EipUint32 instance_number) { +EipBool8 IsConnectedOutputAssembly(const CipInstanceNum instance_number) { EipBool8 is_connected = false; DoublyLinkedListNode *node = connection_list.first; diff --git a/source/src/cip/cipconnectionmanager.h b/source/src/cip/cipconnectionmanager.h index f607a6ef00..a9b303c41b 100644 --- a/source/src/cip/cipconnectionmanager.h +++ b/source/src/cip/cipconnectionmanager.h @@ -199,7 +199,7 @@ CipConnectionObject *GetConnectedOutputAssembly( void CloseConnection(CipConnectionObject *RESTRICT connection_object); /* TODO: Missing documentation */ -EipBool8 IsConnectedOutputAssembly(const EipUint32 instance_number); +EipBool8 IsConnectedOutputAssembly(const CipInstanceNum instance_number); /** @brief Insert the given connection object to the list of currently active * and managed connections. diff --git a/source/src/cip/cipepath.h b/source/src/cip/cipepath.h index b77b14448d..a794e1533a 100644 --- a/source/src/cip/cipepath.h +++ b/source/src/cip/cipepath.h @@ -180,7 +180,7 @@ typedef enum symbolic_segment_extended_format { /* Start - Often used types of EPaths */ typedef struct connection_path_epath { CipDword class_id; /**< here in accordance with Vol. 1 C-1.4.2 */ - CipDword instance_id; + CipInstanceNum instance_id; CipDword attribute_id_or_connection_point; } CipConnectionPathEpath; /* End - Often used types of EPaths */ diff --git a/source/src/cip/cipethernetlink.c b/source/src/cip/cipethernetlink.c index fad3555a1b..91acc2ee15 100644 --- a/source/src/cip/cipethernetlink.c +++ b/source/src/cip/cipethernetlink.c @@ -275,9 +275,9 @@ EipStatus CipEthernetLinkInit(void) { #endif /* bind attributes to the instance */ - for (unsigned int idx = 0; idx < OPENER_ETHLINK_INSTANCE_CNT; ++idx) { + for (CipInstanceNum idx = 0; idx < OPENER_ETHLINK_INSTANCE_CNT; ++idx) { CipInstance *ethernet_link_instance = - GetCipInstance(ethernet_link_class, idx + 1); + GetCipInstance( ethernet_link_class, (CipInstanceNum)(idx + 1) ); InsertAttribute(ethernet_link_instance, 1, @@ -551,7 +551,7 @@ EipStatus GetAndClearEthernetLink( #if defined(OPENER_ETHLINK_IFACE_CTRL_ENABLE) && \ 0 != OPENER_ETHLINK_IFACE_CTRL_ENABLE -static bool IsIfaceControlAllowed(CipUdint instance_id, +static bool IsIfaceControlAllowed(CipInstanceNum instance_id, CipEthernetLinkInterfaceControl const *iface_cntrl) { const CipUsint duplex_mode = diff --git a/source/src/cip/cipmessagerouter.c b/source/src/cip/cipmessagerouter.c index cc705b1717..849c2de62e 100644 --- a/source/src/cip/cipmessagerouter.c +++ b/source/src/cip/cipmessagerouter.c @@ -137,7 +137,7 @@ CipClass *GetCipClass(const CipUdint class_code) { } CipInstance *GetCipInstance(const CipClass *RESTRICT const cip_class, - const EipUint32 instance_number) { + const CipInstanceNum instance_number) { if(instance_number == 0) { return (CipInstance *) cip_class; /* if the instance number is zero, return the class object itself*/ diff --git a/source/src/cip/ciptypes.c b/source/src/cip/ciptypes.c index ceef3c6cda..4e84c15fb3 100644 --- a/source/src/cip/ciptypes.c +++ b/source/src/cip/ciptypes.c @@ -7,6 +7,10 @@ #include #include + +const CipInstanceNum kCipInstanceNumMax = UINT16_MAX; + + /* functions*/ size_t GetCipDataTypeLength(EipUint8 type, const EipUint8 *data) { diff --git a/source/src/cip/ciptypes.h b/source/src/cip/ciptypes.h index 355e763ec6..f367d7dcc9 100644 --- a/source/src/cip/ciptypes.h +++ b/source/src/cip/ciptypes.h @@ -211,6 +211,16 @@ typedef struct cip_type_string_i_struct { CipOctet *string; /**< Pointer to the string data */ } CipStringIStruct; + +/** @brief Highest CIP instance number. + * + * Largest value that can be used to represent or count CIP instances; + * intentended for use when validating instance numbers encoded as data types + * that can contain illegal values. + */ +extern const CipInstanceNum kCipInstanceNumMax; + + /** @brief Struct for padded EPATHs * * Here the class code is referenced as class ID - see Vol. 1 C-1.4.2 @@ -219,7 +229,7 @@ typedef struct cip_type_string_i_struct { typedef struct { EipUint8 path_size; /**< Path size in 16 bit words (path_size * 16 bit) */ EipUint16 class_id; /**< Class ID of the linked object */ - EipUint16 instance_number; /**< Requested Instance Number of the linked object */ + CipInstanceNum instance_number; /**< Requested Instance Number of the linked object */ EipUint16 attribute_number; /**< Requested Attribute Number of the linked object */ } CipEpath; @@ -308,7 +318,7 @@ typedef struct { * pointer of the @ref CipClass structure. */ typedef struct cip_instance { - EipUint32 instance_number; /**< this instance's number (unique within the class) */ + CipInstanceNum instance_number; /**< this instance's number (unique within the class) */ CipAttributeStruct *attributes; /**< pointer to an array of attributes which is unique to this instance */ struct cip_class *cip_class; /**< class the instance belongs to */ diff --git a/source/src/opener_api.h b/source/src/opener_api.h index 29b74c5b8a..08fce6fa5a 100644 --- a/source/src/opener_api.h +++ b/source/src/opener_api.h @@ -213,7 +213,7 @@ CipClass *GetCipClass(const CipUdint class_code); * 0 if instance is not in the object */ CipInstance *GetCipInstance(const CipClass *RESTRICT const cip_object, - const EipUint32 instance_number); + const CipInstanceNum instance_number); /** @ingroup CIP_API * @brief Get a pointer to an instance's attribute @@ -258,7 +258,7 @@ CipClass *CreateCipClass(const CipUdint class_code, const int number_of_instance_attributes, const EipUint32 highest_instance_attribute_number, const int number_of_instance_services, - const int number_of_instances, + const CipInstanceNum number_of_instances, const char *const name, const EipUint16 revision, InitializeCipClass initializer); @@ -283,7 +283,7 @@ CipClass *CreateCipClass(const CipUdint class_code, */ CipInstance *AddCipInstances( CipClass *RESTRICT const cip_object_to_add_instances, - const int number_of_instances); + const CipInstanceNum number_of_instances); /** @ingroup CIP_API * @brief Create one instance of a given class with a certain instance number @@ -296,7 +296,7 @@ CipInstance *AddCipInstances( * */ CipInstance *AddCipInstance(CipClass *RESTRICT const cip_class_to_add_instance, - const EipUint32 instance_id); + const CipInstanceNum instance_id); /** @ingroup CIP_API * @brief Insert an attribute in an instance of a CIP class @@ -559,7 +559,7 @@ int DecodeCipShortString(CipShortString *const data, * The notification on received configuration data is handled with the * AfterAssemblyDataReceived. */ -CipInstance *CreateAssemblyObject(const EipUint32 instance_number, +CipInstance *CreateAssemblyObject(const CipInstanceNum instance_number, EipByte *const data, const EipUint16 data_length); diff --git a/source/src/ports/MINGW/sample_application/ethlinkcbs.c b/source/src/ports/MINGW/sample_application/ethlinkcbs.c index ae88f0fcd0..553db7db01 100644 --- a/source/src/ports/MINGW/sample_application/ethlinkcbs.c +++ b/source/src/ports/MINGW/sample_application/ethlinkcbs.c @@ -66,7 +66,7 @@ EipStatus EthLnkPreGetCallback CipUint attr_no = attribute->attribute_number; /* ATTENTION: Array indices run from 0..(N-1), instance numbers from 1..N */ - CipUdint inst_no = instance->instance_number; + CipInstanceNum inst_no = instance->instance_number; unsigned idx = inst_no-1; switch (attr_no) { case 4: { @@ -133,7 +133,7 @@ EipStatus EthLnkPostGetCallback CipByte service ) { - CipUdint inst_no = instance->instance_number; + CipInstanceNum inst_no = instance->instance_number; EipStatus status = kEipStatusOk; if (kEthLinkGetAndClear == (service & 0x7f)) { diff --git a/source/src/ports/POSIX/sample_application/ethlinkcbs.c b/source/src/ports/POSIX/sample_application/ethlinkcbs.c index ae88f0fcd0..553db7db01 100644 --- a/source/src/ports/POSIX/sample_application/ethlinkcbs.c +++ b/source/src/ports/POSIX/sample_application/ethlinkcbs.c @@ -66,7 +66,7 @@ EipStatus EthLnkPreGetCallback CipUint attr_no = attribute->attribute_number; /* ATTENTION: Array indices run from 0..(N-1), instance numbers from 1..N */ - CipUdint inst_no = instance->instance_number; + CipInstanceNum inst_no = instance->instance_number; unsigned idx = inst_no-1; switch (attr_no) { case 4: { @@ -133,7 +133,7 @@ EipStatus EthLnkPostGetCallback CipByte service ) { - CipUdint inst_no = instance->instance_number; + CipInstanceNum inst_no = instance->instance_number; EipStatus status = kEipStatusOk; if (kEthLinkGetAndClear == (service & 0x7f)) { diff --git a/source/src/ports/WIN32/sample_application/ethlinkcbs.c b/source/src/ports/WIN32/sample_application/ethlinkcbs.c index ae88f0fcd0..553db7db01 100644 --- a/source/src/ports/WIN32/sample_application/ethlinkcbs.c +++ b/source/src/ports/WIN32/sample_application/ethlinkcbs.c @@ -66,7 +66,7 @@ EipStatus EthLnkPreGetCallback CipUint attr_no = attribute->attribute_number; /* ATTENTION: Array indices run from 0..(N-1), instance numbers from 1..N */ - CipUdint inst_no = instance->instance_number; + CipInstanceNum inst_no = instance->instance_number; unsigned idx = inst_no-1; switch (attr_no) { case 4: { @@ -133,7 +133,7 @@ EipStatus EthLnkPostGetCallback CipByte service ) { - CipUdint inst_no = instance->instance_number; + CipInstanceNum inst_no = instance->instance_number; EipStatus status = kEipStatusOk; if (kEthLinkGetAndClear == (service & 0x7f)) { diff --git a/source/src/typedefs.h b/source/src/typedefs.h index 7f2c54561f..b50e8d1b08 100644 --- a/source/src/typedefs.h +++ b/source/src/typedefs.h @@ -68,6 +68,17 @@ static const int kEipInvalidSocket = -1; typedef unsigned long MilliSeconds; typedef unsigned long long MicroSeconds; + +/** @brief CIP object instance number type. + * + * This type is intended for storing values related to identifying or + * quantifying object instances, e.g., the maximum number of instances or + * a specific instance number. The data type is based on @cite CipVol1, + * Table 4-4.2, Attribute IDs 2 and 3. + */ +typedef CipUint CipInstanceNum; + + /** The following are generally true regarding return status: