GameNetworkingSockets
GameNetworkingSockets copied to clipboard
ValveSoftware
build: Fix compilation errors when building with Protobuf 29.0+
Starting from Protobuf 29.0, protobuf::Message::GetTypeName() can return either std::string or absl::string_view, depending on Abseil build options.
For more details, see:
https://github.com/protocolbuffers/protobuf/commit/e13b8e999b3922d0633802c7f90e39af50a31d76
Detect the return type of the method in compile-time and construct a supplementary std::string, from which we can obtain a proper null-terminated buffer and pass it to the internal assertion function, if needed.
Fixes: ValveSoftware/GameNetworkingSockets#370
string_view::data (std, i assume abseil is the same) does not necessarily return zero-terminated strings, which are required by %s
Nice catch! Indeed, it cannot be used this way. Abseil's string_view is exactly the same in this regard. I'll need to rewrite the code to automatically detect the return type of GetTypeName() and construct a null-terminated buffer out of it, if needed.
Force-pushed the branch to fix the problem discovered by @chriku. SteamNetworkingIdentityToProtobuf macro will attempt to perform a compile-time check to see if msg.GetTypeName() returns std::string or not (in this case, any StringViewLike type will do).
This fix uses direct if constexpr + std::is_same_v combination for comparison instead of relying on various protobuf-related macros since it's both easier to read this way and, AFAIU, the macros defined inside the protobuf aren't exposed to the callers of the library.
This seems to be a bit much overkill for an error path that terminates the program anyway. if I'd gotten a (std::, I assume abseil just works similarly) string_view or sometimes string, I would have just have yeeted it into the std::string constructor anyway, which has the nice c_str, making the expression something like: %s", std::string(msg.GetTypeName()).c_str().
@chriku Sure, you're absolutely right. This is an exceptional path, so indeed there's no need for extra typing to avoid a string copy. I'll post a fix a bit later. Thanks for the review!
Force-pushed the branch to implement the suggestion from @chriku to simplify the code. Also added a brief explanation about code safety with respect to the latest C++ standard draft.
c++ lifetimes are always funny, eh? but the new code seems appropriate for the situation.
PS: A small (hopefully helpful) hint: You (@ManManson) sound strangely AI like, be careful that you don't get misidentified and your PRs closed in fear of copyright uncertainties or worse
Yep. That's the reason I've provided a thorough comment so that it doesn't make anyone looking at the code to raise an eyebrow.
I think I've been writing like that long before AIs became manistream. I just happen to like explicit and verbose communication style when participating in public projects. 😄
Is there a reason why this hasn't been merged, since I would like to use the latest version of Protobuf? Additional note: This would also fix the linux CI pipeline failing.
This fixed my issues compiling this morning on Arch. This PR should be merged.
Sorry, I've been away from this project for a while and I didn't see this. I checked in a separate fix. 096c19525b8d2a9b897a6e668c9612eff750b4c7
