As a side-effect, DHT now always accepts LAN discovery packets, even
when LAN discovery is disabled. When LAN discovery is disabled, those
packets are ignored.
These don't test anything that isn't covered by higher level tox tests.
These are also not unit tests and have never found any bug that wasn't
also caught by other tests. This makes them a pure maintenance burden.
These were found by the new cimple type check which is completely
unforgiving to implicit boolean conversions. After this PR, there should
be no more implicit int-to-bool or bool-to-int conversions.
These were found by the new stronger type check in cimple. The one
bugfix is in `crypto_sha512_cmp`, which used to think `crypto_verify_32`
returns bool while actually it's -1/0/1.
This uses mallocfail to further increase coverage using the existing
tests. Also:
* Moved the non-auto "tox_one_test" to gtest. This should be
split into smaller tests later.
* Changed `hole_punching` to `bool`.
Missed a few of those in check-c. check-cimple now catches these with a
stronger type check.
Other changes:
* `ptr + int` must always have the `ptr` first, so `int + ptr` is not
allowed anymore.
* `close` and `time` were shadowing libc functions. `file_data` was
shadowed in a function (and is not a good function name anyway), so
renamed to `send_file_data` which is more descriptive.
* Within a function, all local variables of the same name must have the
same type.
* The `strerror_r` change wasn't necessary, but I kept it because it
seems a bit clearer to me now. `#ifdef`s inside functions are a bit
confusing sometimes.
Previously we would try to send three random TCP relays that we're
connected to to each friend once every 5 minutes. The problem with
this method is that it could take an extraordinarily long time
to share every relay; some relays might be consistently skipped
while others might be sent repeatedly. Moreover, there's no
guarantee that the nodes you try to send are actually online.
This leads to a prety unreliable and flaky way of sharing.
Now we reduce the timer to two minutes, and cycle through the list
trying 3 nodes each share attempt. This guarantees that every online
node in our list gets shared with every friend after a fixed amount of
time (which depends on how many nodes are in the list)
One of these was creating a single 262144 byte stack frame. We now have
a way to check and limit the allocation size of a VLA. The `Cmp_Data`
ones were also fairly large. Now, no allocation is larger than 2KiB
(though rtp.c allocates close to that much).
This commit adds functionality for clients to interact with
the DHT, sending getnodes requests to their peers and receiving
nodes in getnodes responses.
These help static analysis and ubsan. We should eventually have all
functions annotated like this with a cimple check to make sure every
pointer has an explicit nullability annotation. The `nullable`
annotation does nothing in GCC, but will be used by cimple to validate
that every parameter has defined nullability.
It was kind of thread-safe, maybe, but there was a data race that makes
tsan unhappy. We now do interface detection once per Tox instance
instead of once per process.
Instead of synchronously handling events as they happen in
`tox_iterate`, this first collects all events in a structure and then
lets the client process them. This allows clients to process events in
parallel, since the data structure returned is mostly immutable.
This also makes toxcore compatible with languages that don't (easily)
support callbacks from C into the non-C language.
If we remove the callbacks, this allows us to add fields to the events
without breaking the API.
* Function arguments must use `foo_cb *p` and can't just use `foo_cb p`
* You can no longer cast function pointers (if it's incompatible, you
must wrap the callback). I'm avoiding this with tokstyle exclusions.
Most system headers contain functions (e.g. `memcpy` in `string.h`)
which aren't needed in our own header files. For the most part, our own
headers should only include types needed to declare our own types and
functions. We now enforce this so we think twice about which headers we
really need in the .h files.
strerror_r() has two versions: GNU-specific and XSI-compliant. The XSI
version always stores the string in the provided buffer, but the GNU
version might store it in the provided buffer or it might use some
immutable static buffer instead. Since we always free the error string,
we might end up freeing the immutable static buffer.
Also added a whole bunch of logging that I needed while debugging the
issue. The solution in the end is that bootstrap needs to resolve IPs,
and getaddrinfo fails in the browser. Most of the time we bootstrap
against IPs anyway, so trying to parse as IP address first will shortcut
that.
The brackets serve no purpose and make us do extra string
parsing when using the output for other things
Also removed a useless call to ip_ntoa in LAN_discovery.c
We still have them in toxav. That will need to be cleaned up later.
Flexible array members have very limited usefulness. In this particular
case, it's almost entirely useless. It confuses static analysers and is
yet one more C feature we need to understand and support. It is also the
only reason we need special support in tokstyle for calloc with a `+`
operator in the member size.
Use of `strcpy` in these particular cases was safe, but it's hard to
tell and also useless. `strcpy` would effectively need to do another
`strlen` which we already did.
Also removed sprintf, which was also safe in this case but it's easier to
be "obviously safe", especially for static analysers.
We have a more portable wrapper that is now also thread-safe. Also
stopped using sprintf in the one place we used it. This doesn't really
help much, but it allows us to forbid sprintf globally.
Currently: 1) libsodium and 2) nacl.
Note that the "nacl" variant is actually libsodium. We just want to make
sure the static analysers see the `VANILLA_NACL` code paths.
Also added a valgrind build to run it on every pull request. I've had to
disable a few tests because valgrind makes those run infinitely slowly,
consistently timing them out.
atoi doesn't check if the conversion from string to int succeeded
which doesn't allow us to do proper error handling.
We also now make sure that the port argument is a valid port
in addition to being properly converted
