-
Notifications
You must be signed in to change notification settings - Fork 0
Description
We should consider changing the initial client-server handshake to be on top of protobuf messages to prevent potential cross-platform issues down the road.
Original discussion below:
@JoshStoddard:
Raw byte casting at the API boundary makes me nervous. It looks like client and server are always on the same platform, so byte representations should match. Still, this is very brittle, and it can get real nasty if, for example, the API versions get out of sync.
FWIW, protobuf is purpose-built for this, and you're already using it in eg. TokenBasedVDMSClient::query. With protobuf you get builtin schema validation, platform independence, and backward compatibility; as well as a bunch of guardrails and serviceability hooks. I know you've already built the API and it would be nontrivial effort to retool, but I think the safety and scalability we'd gain by porting to protobuf is worth considering in the near term.
It's worth discussing it. The idea here was to keep it very simple and lightweight.
An alternative, to make sure memory layout matches cross-platform, would be to static assert the size of HelloMessage and even its members to make sure sizes match.
I don't anticipate issues for now unless support for architectures, like arm64, is added where some type sizes differ.