Potential pointer/address bugs

[widavies]

Shouldn't this line be:

const EipUint8 ** cip_message = &message_router_request->data;

instead of:

const EipUint8 ** cip_message = message_router_request->data;

Additionally, the same case for this line

Thanks, I can submit a pull request if needed.

[MartinMelikMerkumians]

Hi,

yes, I think you are right. Seems it is "fixed" by later using the & operator on it...?

Thanks, would be great if you can provide a PR.

Best regards

[widavies]

Sure, I'd love to.

One thing to clarify first - You create a double pointer to the uint8_t data in message_router_request->data.

My understanding is that as you decode data, you extract the data data from the buffer, then increment the buffer forward.

For example (taken from DecodeCipString):

const EipUint8 ** cip_message = &message_router_request->data;
*cip_message += string->length;

I've been staring at this line for awhile, but wouldn't it technically be equivalent to just:

message_router_request->data += string->length;

I'm not sure if instead the double pointer is supposed to get incremented forward or not, but some insight on this would be great.

Thanks for the great library, just ported to LWIP/no-RTOS on ARM and it's working smoothly.

[widavies]

Adding this line too as a reminder https://github.com/EIPStackGroup/OpENer/blob/cef2833fcf2ea9fda68f85c4638429956128daf5/source/src/cip/cipassembly.c#L160

[MartinMelikMerkumians]

Hmm, I think you are absolutely right. Could be that when I changed stuff on the MRR implementation, I went with the fastest, but probably not the clearest way to update the code 😅

[widavies]

No worries, it doesn't look like the functions are even used anyway, they're more just helper functions than anything.

[jvalenzuela]

This has been corrected in the set of commits I'm submitting for addressing compiler warnings, and is also a rather good example why warnings should not be ignored and allowed to accumulate. Here the compiler identified an actual problem, but was masked by the overwhelming number of nuisance warnings.

[MartinMelikMerkumians]

Should be solved by now