OpENer icon indicating copy to clipboard operation
OpENer copied to clipboard

Serialization buffers

Open jvalenzuela opened this issue 2 years ago • 2 comments

I've noticed there are several types of memory buffers used to store raw, serialized data either received from the network or being encoded for transmission onto the network. Some examples are CipMessageRouterRequest.data and ENIPMessage. In addition to different storage types, there are many more various forms of accessing data within these buffers, such as:

  • GetXXXFromMessage: directly advances a pointer to data.
  • SetCipStringXByData: reads from a data pointer, but doesn't advance it.
  • AddXXXToMessage: Alters a buffer struct with internal index and pointers.
  • EncodeCipXXX: wrapper for AddToMessage to conform to a specific call signature.
  • Direct(in-line) buffer pointer manipulation.

All these are basically different ways of doing the same thing, and there are a lot of compiler warnings and just plain bugs due to casting away consts, pointer errors, and buffer overflow potentials due to incorrectly formed packets(e.g. assuming a received string contains a valid length).

Would you be interested in refactoring all raw buffer operations into a single, dedicated module that handles all the details associated with buffer management? External code can only get values from, and put values into buffers via function calls which refer to an opaque buffer data type. The actual buffer implementation, i.e. current get/put position maintenance, bounds checking, etc, will be hidden in the buffer module, and have one, uniform implementation(with unit tests).

jvalenzuela avatar Jan 18 '22 01:01 jvalenzuela

An initial pass on an API is in the buf branch of my fork: https://github.com/jvalenzuela/OpENer/tree/buf. Doxygen documentation is included, so it can be reviewed via make doc.

jvalenzuela avatar Jan 25 '22 21:01 jvalenzuela

Hi, sorry for the long delay. Yes, I would be interested to clean this up. The API has been created and changed over and over again to reduce code duplications and errors resulting in not moving the pointer, while unfortunately at some instances I needed exactly the non-moving behavior.

MartinMelikMerkumians avatar Oct 04 '22 10:10 MartinMelikMerkumians