Ryan-rsm-McKenzie / bsa

C++ library for working with the Bethesda archive file format
MIT License
41 stars 6 forks source link

`file::write` accesses null pointer when default constructed #12

Closed Guekka closed 1 year ago

Guekka commented 1 year ago

This is due to how byte_container is defined. By default, it creates an empty span https://github.com/Ryan-rsm-McKenzie/bsa/blob/938c6b5c960f5ba9c4f457f34a41665fd38023e0/include/bsa/detail/common.hpp#L568-L572 Obviously, std::data of this empty span will return nullptr.

By making data_owner the first alternative, we could avoid this issue. Or maybe default construction should be a span to a static empty vector, for performance?

If this is by design, I suggest adding an assertion to make debugging easier.

Ryan-rsm-McKenzie commented 1 year ago

It seems to me like this is more an issue with binary_io not checking if the span is empty before writing: https://github.com/Ryan-rsm-McKenzie/binary_io/blob/3c473be519e5ca6889e1faa7d383e0c3dbb30e1b/src/binary_io/binary_io.cpp#L264-L273

Guekka commented 1 year ago

It seems to me like this is more an issue with binary_io not checking if the span is empty before writing: https://github.com/Ryan-rsm-McKenzie/binary_io/blob/3c473be519e5ca6889e1faa7d383e0c3dbb30e1b/src/binary_io/binary_io.cpp#L264-L273

Indeed, that's a better place to fix

Ryan-rsm-McKenzie commented 1 year ago

Fixed by microsoft/vcpkg#32938

Ryan-rsm-McKenzie commented 1 year ago

Let me know if this fixes the problem

Guekka commented 1 year ago

Yes, it does. Thanks!