These are more convenient and safer than the manual vtables we have in
the fuzzer support code. We can override individual member functions,
and C++ will take care of correctly casting and offsetting this-pointers
when needed.
We allow non-null data pointers to be passed to functions
alongside 0-length data. For example when creating a data
buffer that has room for the entire packet, including ignored
header data.
This error broke a rare but legitimate case where we miss
packets during a handshake attempt and need to store empty
handshake packets in the packet array.
So we don't need to write so many edge case tests ourselves for things
like parsers, which really don't need those manual tests, as long as we
can check for some properties like "can output the parsed data and it'll
be the same as the input".
commit 5b9c420c introduced some undesirable behaviour with packet send
functions returning error when they shouldn't. We now only return an
error if the packet fails to be added to the send queue or cannot
be wrapped/encrypted. We no longer error if we fail to send the packet
over the wire, because toxcore will keep trying to re-send the packet
until the connection times out.
Additionally, we now make sure that our packet broadcast functions
aren't returning an error when failing to send packets to peers
that we have not successfully handshaked with yet, since this is
expected behaviour.
We always assumed that create_array_entry() would only be called
with an empty array entry and wouldn't modify entries on error.
We now explicitly require both conditions, and also give an
error in the case of a non-null data pointer with a zero
length field, as this indicates a logic error.
Checks for an empty array entry that precede a call to
create_array_entry() are now redundant. It should be noted that
a non-empty entry doesn't necessarily indicate an error. This
condition can be triggered if packets are being sent or
received faster than they can be processed/acknowledged,
which is common when spamming messages on poor connections.
Avoiding passing down the entire DHT struct pointer to the inner
functions makes it possible in the future to write unit tests without
having to construct a full DHT object.
None of the others use out parameters. Also no toxcore function uses out
parameters for anything other than arrays and errors. This would be a
first, for no good reason.
These make it clearer which kinds of `uint32_t` can fit in which
functions. It also makes it easier to generate higher level bindings
without doing a lot of inference/heuristics.
This fetches it from github, so we don't need to build it locally.
Not super ideal, because devs are supposed to build it locally to prove
reproducibility, but we can keep that diligence on the dev to do once
when actually merging the PR.
This fixes time resolution issues and simplifies the code a bit. QPC can
in theory jump forward in time, but in practice not by enough to matter
in our use case.
Also started teaching it about toxcore's alloc/dealloc functions in
hopes of it catching some errors (it doesn't seem to be very good at
this, but maybe better than nothing?).
Instead of using `target_link_modules`, which does magic that we no
longer need, because we only have 1 library we install, and all binaries
we build link statically because they need access to internal symbols.
We now depend on libsodium unconditionally. Future work will require
functions from libsodium, and nobody we're aware of uses the nacl build
for anything other than making sure it still works on CI.
