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.
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.
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.
This function will return an IP address string associated with a peer.
If the peer is not accepting direct connections a placeholder value
will be returned, indicating that their real IP address is unknown.
We do not return TCP relay IP addresses because a TCP connection
with a peer may use multiple relays simultaneously.
This mainly saves spam in test logs, but may save some packets here and
there, if nodes are randomly selected twice for GET_NODES and onion
routing packets.
Rather than aborting the process on invalid group save data we
either try to continue if possible, or abort the saving/loading
instead of the entire process
memset() treats the passed buffer as a char* array, assigning to every
1-byte of the array the value. So for a single 4-byte int32_t element,
it is assigning bytes 0, 1, 2 and 3 of it to -1. It happens that -1 is
0xFF, so in the end the uint32_t is set to 0xFFFFFFFF, which is -1 in
the two's complement, so the memset() actually produces the correct
result in the end, assuming the platform uses two's complement integers.
Assigning it in the loop is less error-prone, as using memset() on
non-1-byte wide arrays with a non-zero value is fishy, and it is more
portable as we don't have to assume the use of two's complement.
It looks like in a future version of the C standard, C23, two's
complement is the only integer format in C23 (thanks to @robinlinden on
IRC for pointing that out), so perhaps we shouldn't be as concerned with
the portability here? Though @iphydf says that it's still a good idea to
use a for-loop for this case.
It doesn't work at all, because we're missing something in the net code
to do with endian conversions. I haven't investigated, yet, but at least
now we have a failing test that can be investigated.
Also moved to cmake 3.5 at minimum. CMake will stop supporting lower
versions than that, soon.
Also moved to C11 from C99 to get `static_assert`.
Also made a network ERROR into a WARNING. It triggers on FreeBSD.
