Increase integer width for CPF address item count. #463
Conversation
source/src/enet_encap/cpf.c
Outdated
| @@ -271,7 +271,7 @@ EipStatus CreateCommonPacketFormatStructure(const EipUint8 *data, | |||
| return kEipStatusError; | |||
| } | |||
|
|
|||
| CipUsint address_item_count = common_packet_format_data->item_count - 2; | |||
| unsigned int address_item_count = common_packet_format_data->item_count - 2U; | |||
There was a problem hiding this comment.
why do you want to increase the width? the specification states its a USINT
There was a problem hiding this comment.
Oh sorry, you are right. its an UINT.
Could we please use CipUint to create a direct match with the specification?
It helpes me a lot to keep track which variables are part of the interface, and which are just part of the implementation
|
The spec I have, Volume 2, 1.4, section 2-6.1 says it item count is a UINT. Regarding the use of unsigned int instead of CipUint, cpf_data->item count(EipUint16) has a rank equal to or lower than unsigned int on any machine, so it will be implictly promoted to unsigned int. Now the right side of the assignment will yield an unsigned int, which causes a warning due to implicit conversion to a smaller integer with gcc -Wconversion, unsigned int -> CipUsint or CipUint. Since the address_item_count isn't going to be serialized, I went with a machine-dependent unsigned int as it is guaranteed to handle UINT, and matches the promoted result of cpf_data->item_count - 2U. Maybe adding a comment conveying the above justification would be merited. Another idea would be to use CipUint and cast the expression: |
Resolves warning about possible loss of data due to implicit integer conversion, e.g., Visual Studio C4244.
No description provided.