Apollo3zehn
commented
3 months ago Thanks for bringing this up. I agree that a possible return value should be read-only and that there is a need to pass your own buffer. I will put this on my v6 release list and try to work on it soon together with other API design changes (endianess handling).
Related to #52
TL;DR - Exposing the underlying buffer smells wrong. (IMO)
The current client API frequently returns Span or Memory, which ultimately are slices into the buffer owned by the ModbusFrameBuffer type. Subsequent operations involving that buffer are directly observable by consuming code; a return value from one of the read methods is always 'corrupted' by a subsequent read/write. Furthermore, the existing return types enable the underlying buffer to be modified by external code, potentially corrupting the buffer whilst it is being used by the API. (were this activity to be performed asynchronously) At a minimum, ReadOnlySpan and ReadOnlyMemory ought to be returned instead of Span and Memory, respectively, but it would be better to avoid exposing the internal buffer altogether. In my opinion, it would be preferable for the read methods to expect a Span or Memory argument instead; the same memory block would still be affected were the same or overlapping block supplied to multiple read methods in succession, but this would be significantly less surprising for the consuming code. I appreciate that this would involve an additional copy operation, but I believe it would be a significantly cleaner design.