fheroes2 icon indicating copy to clipboard operation
fheroes2 copied to clipboard

Implement a type-safe and const-correct version of StreamBuf class

Open oleg-derevenetz opened this issue 5 months ago • 0 comments

Currently in the master branch, the StreamBuf class has a major flaw: if it is created to get access to the existing std::vector<uint8_t>::data() backend, it uses the const_cast to cast away the const'ness from the vector's data. This is very close to undefined behavior and is very dangerous if methods are accidentally called that will write to this buffer (like put8()/putRaw()/etc).

This PR turns StreamBuf into an abstract class, so various functions, methods, and operators can still use it by reference, which ensures that changes in the code using it are minimal. But, if you need to create a StreamBuf instance, you cannot do this anymore. Instead, you now have two separate classes at your disposal:

  • RWStreamBuf (with writable uint8_t * backend managed by the RWStreamBuf itself);
  • ROStreamBuf (with read-only const uint8_t * backend which points to the data() of the underlying std::vector<uint8_t>).

Both of these classes have an intermediate ancestor in the form of the StreamBufTmpl class template, which removes code duplication while providing type safety and const-correctness in a natural way via C++ type system.

It would be even more correct to make separate interfaces for reading and writing (just like std::istream and std::ostream) instead of single StreamBase, but this will require a very large number of changes in many places, because the current I/O code (all these operator>>() and operator<<(), etc, etc) is not designed for this right now. Maybe later in a separate PR.

oleg-derevenetz avatar Aug 26 '24 22:08 oleg-derevenetz