Did my best to surmise the size requirements of
these integers, will do the rest of the tests soon. Also added a todo
and made an obsessive change to a for loop.
* Moved PAIR to toxav, where it's used (but really this should die).
* Replace most MIN calls with typed `min_*` calls. Didn't replace the
ones where the desired semantics are unclear. Moved the MIN macro to
the one place where it's still used.
* Avoid assignments in `while` loops. Instead, factored out the loop body
into a separate `bool`-returning function.
* Use named types for callbacks (`_cb` types).
* Avoid assignments in `if` conditions.
* Removed `MAKE_REALLOC` and expanded its two calls. We can't have
templates in C, and this fake templating is ugly and hard to analyse
and debug (it expands on a single line).
* Moved epoll system include to the .c file, out of the .h file.
* Avoid assignments in expressions (`a = b = c;`).
* Avoid multiple declarators per struct member declaration.
* Fix naming inconsistencies.
* Replace `net_to_host` macro with function.
* Removed `ARRAY_SIZE` and use NULL markers for end of array, instead.
The alternative is + size, but for these arrays, NULL markers made
sense, since they are arrays of non-null pointers.
* Made `INDEX_OF_PK` a self-contained macro, not dependent upon the
naming inside its call site. This is a minor change but makes the code
more local and reviews easier.
* No nested structs.
* Use only named function types ending in `_cb` for callbacks.
* Replaced two macros with functions.
* `++i` instead of `i++`.
* struct member names start with lowercase letters.
* It takes a bit of work to support `/**/` comments in preprocessor
macros, so I've decided not to support these. If a macro is complex
enough to need comments inside it, it's too complex. `//` comments are
allowed at the end of macro definitions.
* Callback typedefs must name their parameters.
It turns out, `unix_time` is also monotonic, and is used as such, so I've
renamed the new functions to `mono_time_*`.
2018-07-08:
```
00:01 <@irungentoo> the idea used to be that the unix_time() function
could go backward in time but I think I might have started using it like
if it could not after I changed it so that it would never go back in time
```
Also, renamed simple_conference_test to conference_simple_test so it's
sorted together with the other conference tests.
Next step is to use run_auto_test.h for the conference test.
Removed a pointless declaration of a function in lan_discovery_test
and cleaned up the one error message there. Did an entire restructuring
of the version_test using macros that resulted in fewer lines of code but more
thorough testing.
Formatting of version_test.c
back to old way, save comments and one change
Missing space
My greatest enemy
Add `#include <cstdio>` for `std::printf`.
Make tox.c unambiguously parseable.
Rules:
1. Constants are uppercase names: THE_CONSTANT.
2. SUE[1] types start with an uppercase letter and have at least one
lowercase letter in it: The_Type, THE_Type.
3. Function types end in "_cb": tox_friend_connection_cb.
4. Variable and function names are all lowercase: the_function.
This makes it easier for humans reading the code to determine what an
identifier means. I'm not convinced by the enum type name change, but I
don't know a better rule. Currently, a lot of enum types are spelled like
constants, which is confusing.
[1] struct/union/enum
Use run_auto_test.h test fixture for some auto-tests.
Most of the auto-tests should use this fixture, but I've only done a few
to set an example.
Rules:
1. Constants are uppercase names: THE_CONSTANT.
2. SUE[1] types start with an uppercase letter and have at least one
lowercase letter in it: The_Type, THE_Type.
3. Function types end in "_cb": tox_friend_connection_cb.
4. Variable and function names are all lowercase: the_function.
This makes it easier for humans reading the code to determine what an
identifier means. I'm not convinced by the enum type name change, but I
don't know a better rule. Currently, a lot of enum types are spelled like
constants, which is confusing.
[1] struct/union/enum
These display some idea, but the tests are not implemented correctly. We
will need to implement the idea correctly later, but for now we can't use
these.
Mostly documentation + comments. Some cases where code was removed
in exchange for more compact/less sprawly for loops. Introduced a
function that removed like 30 lines of repeated code.
The C compiler warns because the value is initialised in a loop and used
outside of it. In this case, it's always initialised, but changing the
value of `NUM_PORTS` can change that.
Better error messages, better sleep() call placements, etc.
Did not modify large chunk of function because I couldn't explain
it. Maybe I'll come back later once I've regained lost brain cells.
This triggers a code path in Persistent Group Chats that causes a memory
leak. I'm adding this test now, so that we don't merge PGC without fixing
the memory leak first.
This is needed for libvpx to work on android.
This also means that we can upload our test binaries to an android device
and actually run them, now that libcheck is no longer a blocker.
Also, add an auto-test for bootstrap and for LAN discovery.
Bootstrap is never tested otherwise, and LAN discovery is a prerequisite
for everything else. Having these two tests lets us rule out or identify
LAN discovery as a possible cause for test failures.
Also reduce number of people in conference to 5, because on Circle CI the
test times out trying to connect more than 6 or 7 people. The persistent
conferences PR will improve this so we can set it much higher then.
follow TokTok#731. This commit
completely removed all things in namespace bit_rate, and deprecated
functions are to be added back in another commit. set_xxx() is treadted
as a property of namespace audio&video, same as bit_rate change event.
toxav_basic_test is fixed, either.
Also got rid of two VLAs. They are overused a bit in toxcore. In
irc_syncbot, the array was uninitialised and then filled by a recv system
call. This can cause uninitialised reads if recv doesn't fill the entire
array. It could not cause out of bounds read directly, because a
NUL-terminator was in place, but both cases are undefined behaviour.
Fixes#572.
As discussed in the issue, there's a risk that toxcore may not hold the
maximum bitrates libvpx supports, if toxcore insists on using integer
type. I initially proposed to have another flag in set(), so that we can
use unsigned type instead. iphydf came up with a better solution, that is
splitting the original functions, one for audio, one for video. Now, we
could safely replace int32_t with uint32_t.
Also: clean video_bit_rate_invalid()
Though this is not a part of issue #572, as it's used in the
toxav_bit_rate_set(), i cleaned the code. As mannol said, there should be
a check. Uint32_t is large enough to hold the maximum bitrates libvpx
supports, but user may pass a value larger than uint while smaller than
uint32_t. Thanks to the reminding from nurupo, it's no longer a stub
function.
Bitrate error enums are shared for both audio and video
https://github.com/TokTok/c-toxcore/pull/578#issuecomment-360095609, just
as iphydf said.
This way, developers compile toxcore, toxav, and toxencryptsave as C++ at
least once at home, reducing the likelyhood of running into travis
failures where we compile as C++ in the windows build.
This allows us and users to reproducibly build verified versions of the
library with checksums. It will power the toktok-stack continuous build
with checked-in checksums at specific git revisions.
Tests are not actually ran on appveyor for now, since they all fault for
some reason. For now, we just build them. Also, some tests are disabled
on msvc entirely, because they don't even compile. We'll need to look
into those, later. They are disabled using `MSVC_DONT_BUILD`.
This requires that every symbol, even if static (file-scope), is unique.
The idea is that we can easily run "whole" program static analysis on
programs that include monolith.h ("whole" is in quotes, as we don't
include dependencies like libsodium in this static analysis).
- rework ip_ntoa() to avoid use of static variables
- rework sort_client_list() to avoid use of static variables
- move static 'lastdump' into Messenger struct
- rework ID2String() to avoid use of static variables; rename to id_to_string()
- fetch_broadcast_info(): attempt to mitigate risks from concurrent execution
- current_time_monotonic(): attempt to mitigate risks from concurrent execution
- comment on non-thread-safety of unix_time_update
"All rights reserved" was incorrect. The project was licensed under GPL3,
which means a lot of rights are licensed to everybody in the world, i.e.
not reserved to the "Tox Project".
- CFLAG gnu99 was changed to c99.
- CXXFLAG c++98 was changed to c++11.
- CFLAG -pedantic-errors was added so that non-ISO C now throws errors.
- _XOPEN_SOURCE feature test macro added and set to 600 to expose SUSv3
and c99 definitions in modules that required them.
- Fixed tests (and bootstrap daemon logging) that were failing due to
the altered build flags.
- Avoid string suffix misinterpretation; explicit narrowing conversion.
- Misc. additions to .gitignore to make sure build artifacts don't wind
up in version control.
Also added a `tox_options_copy` function for cloning an options object.
This can be useful when creating several Tox instances with slightly
varying options.
We create and destroy 20k tox instances and run a single tox_iterate on
it. This test is not comprehensive, but provides a simple check to see
whether the destruction properly cleans up memory and perhaps other
resources.
`new_nonce` has been an alias for `random_nonce` for a while now. Having
two names for the same operation is confusing. `random_nonce` better
expresses the intent. The documentation for `new_nonce` talks about
guaranteeing that the nonce is different from previous ones, which is
incorrect, it's just quite likely to be different.
Previously, all log messages generated by tox_new (which is quite a lot)
were dropped, because client code had no chance to register a logging
callback, yet. This change allows setting the log callback from the
beginning and removes the ability to unset it.
Since the log callback is forever special, since it can't be stateless,
we don't necessarily need to treat it uniformly (with `event`).
We disable the ones that fire, so we can use -Werror. We can then
investigate each warning individually and see whether to fix it or to
keep silencing it.
Compiling as C++ changes nothing semantically, but ensures that we don't
break C++ compatibility while also retaining C compatibility.
C++ compatibility is useful for tooling and additional diagnostics and
analyses.
In a next step, we will remove tests from each file to have a per-binary
split of tests. This will help identify which tests fail most often on
Travis CI.
In another future step, we will split the large one_test into several
auto tests, which will make testing quite a bit slower (adding about 10
seconds setup time to each), but hopefully a lot more stable ("Tox went
offline" should not happen as much anymore).
It is still C code, so still compatible with C compilers as well. This
change lets us see more clearly where implicit conversions occur by
making them explicit.
- All global variables should be static unless they have an explicit
extern declaration in a header file.
- `to_compare` was not used in encryptsave and toxav tests.
- `break` in switch cases is not required directly after `return`,
`goto`, or a noreturn function like `abort`.
