From c08b2fb3e2c3c8e8a5722e7350b61efd6992be45 Mon Sep 17 00:00:00 2001 From: iphydf Date: Thu, 9 Apr 2020 23:56:19 +0000 Subject: Remove tokstyle exemptions from build files. We put some tokstyle exemptions into the source files themselves, instead. This way we can check some of the code in those files, and more in the future when tokstyle supports more constructs (like apidsl). Also: hacked ping_array.api.h to not emit `_array` as parameter names. We'll need to fix apidsl to do this better. This works for now. --- toxav/BUILD.bazel | 7 ++-- toxav/toxav.api.h | 4 +++ toxav/toxav.h | 4 +++ toxcore/BUILD.bazel | 9 +---- toxcore/ccompat.h | 4 +++ toxcore/crypto_core.api.h | 2 +- toxcore/crypto_core.h | 2 +- toxcore/crypto_core_mem.c | 26 +++++++------- toxcore/crypto_core_test.cc | 40 ++++++++++------------ toxcore/ping_array.api.h | 4 +-- toxcore/ping_array.h | 6 ++-- toxcore/tox.api.h | 4 +++ toxcore/tox.h | 4 +++ toxcore/tox_api.c | 3 ++ .../pwhash_scryptsalsa208sha256.c | 2 +- toxencryptsave/defines.h | 2 +- 16 files changed, 67 insertions(+), 56 deletions(-) diff --git a/toxav/BUILD.bazel b/toxav/BUILD.bazel index 33956aac..036bde3f 100644 --- a/toxav/BUILD.bazel +++ b/toxav/BUILD.bazel @@ -45,8 +45,8 @@ cc_library( hdrs = ["bwcontroller.h"], deps = [ ":ring_buffer", + "//c-toxcore/toxcore", "//c-toxcore/toxcore:Messenger", - "//c-toxcore/toxcore:toxcore", ], ) @@ -130,10 +130,7 @@ CIMPLE_SRCS = glob( "*.c", "*.h", ], - exclude = [ - "*.api.h", - "toxav.h", - ], + exclude = ["*.api.h"], ) sh_test( diff --git a/toxav/toxav.api.h b/toxav/toxav.api.h index 158f74e6..269fd3d7 100644 --- a/toxav/toxav.api.h +++ b/toxav/toxav.api.h @@ -10,6 +10,8 @@ #include #include +//!TOKSTYLE- + #ifdef __cplusplus extern "C" { #endif @@ -686,5 +688,7 @@ typedef TOXAV_ERR_BIT_RATE_SET Toxav_Err_Bit_Rate_Set; typedef TOXAV_ERR_SEND_FRAME Toxav_Err_Send_Frame; typedef TOXAV_CALL_CONTROL Toxav_Call_Control; +//!TOKSTYLE+ + #endif // C_TOXCORE_TOXAV_TOXAV_H %} diff --git a/toxav/toxav.h b/toxav/toxav.h index 141ccb89..a96c85cb 100644 --- a/toxav/toxav.h +++ b/toxav/toxav.h @@ -9,6 +9,8 @@ #include #include +//!TOKSTYLE- + #ifdef __cplusplus extern "C" { #endif @@ -815,4 +817,6 @@ typedef TOXAV_ERR_BIT_RATE_SET Toxav_Err_Bit_Rate_Set; typedef TOXAV_ERR_SEND_FRAME Toxav_Err_Send_Frame; typedef TOXAV_CALL_CONTROL Toxav_Call_Control; +//!TOKSTYLE+ + #endif // C_TOXCORE_TOXAV_TOXAV_H diff --git a/toxcore/BUILD.bazel b/toxcore/BUILD.bazel index 2a9b4ff2..403fa7e8 100644 --- a/toxcore/BUILD.bazel +++ b/toxcore/BUILD.bazel @@ -286,14 +286,7 @@ CIMPLE_SRCS = glob( "*.c", "*.h", ], - exclude = [ - "*.api.h", - "ccompat.h", - "crypto_core_mem.c", - "ping_array.h", - "tox.h", - "tox_api.c", - ], + exclude = ["*.api.h"], ) sh_test( diff --git a/toxcore/ccompat.h b/toxcore/ccompat.h index ba8c7fe8..8ff6563d 100644 --- a/toxcore/ccompat.h +++ b/toxcore/ccompat.h @@ -4,6 +4,8 @@ #ifndef C_TOXCORE_TOXCORE_CCOMPAT_H #define C_TOXCORE_TOXCORE_CCOMPAT_H +//!TOKSTYLE- + // Variable length arrays. // VLA(type, name, size) allocates a variable length array with automatic // storage duration. VLA_SIZE(name) evaluates to the runtime size of that array @@ -48,4 +50,6 @@ #define GNU_PRINTF(f, a) #endif +//!TOKSTYLE+ + #endif // C_TOXCORE_TOXCORE_CCOMPAT_H diff --git a/toxcore/crypto_core.api.h b/toxcore/crypto_core.api.h index 44f541fc..0db3a420 100644 --- a/toxcore/crypto_core.api.h +++ b/toxcore/crypto_core.api.h @@ -69,7 +69,7 @@ const CRYPTO_SHA512_SIZE = 64; * "aaaa" and "baaa" also takes 4 time. With a regular `memcmp`, the latter may * take 1 time, because it immediately knows that the two strings are not equal. */ -static int32_t crypto_memcmp(const void *p1, const void *p2, size_t length); +static int32_t crypto_memcmp(const uint8_t *p1, const uint8_t *p2, size_t length); /** * A `bzero`-like function which won't be optimised away by the compiler. Some diff --git a/toxcore/crypto_core.h b/toxcore/crypto_core.h index 07412e7e..425c3244 100644 --- a/toxcore/crypto_core.h +++ b/toxcore/crypto_core.h @@ -83,7 +83,7 @@ uint32_t crypto_sha512_size(void); * "aaaa" and "baaa" also takes 4 time. With a regular `memcmp`, the latter may * take 1 time, because it immediately knows that the two strings are not equal. */ -int32_t crypto_memcmp(const void *p1, const void *p2, size_t length); +int32_t crypto_memcmp(const uint8_t *p1, const uint8_t *p2, size_t length); /** * A `bzero`-like function which won't be optimised away by the compiler. Some diff --git a/toxcore/crypto_core_mem.c b/toxcore/crypto_core_mem.c index b8f4223e..b8c0bd9b 100644 --- a/toxcore/crypto_core_mem.c +++ b/toxcore/crypto_core_mem.c @@ -37,8 +37,7 @@ void crypto_memzero(void *data, size_t length) { #ifndef VANILLA_NACL sodium_memzero(data, length); -#else -#ifdef _WIN32 +#elif defined(_WIN32) SecureZeroMemory(data, length); #elif defined(HAVE_MEMSET_S) @@ -53,32 +52,33 @@ void crypto_memzero(void *data, size_t length) #elif defined(HAVE_EXPLICIT_BZERO) explicit_bzero(data, length); #else - volatile unsigned char *volatile pnt = - (volatile unsigned char *volatile) data; + //!TOKSTYLE- + volatile uint8_t *volatile pnt = data; + //!TOKSTYLE+ size_t i = (size_t) 0U; while (i < length) { - pnt[i++] = 0U; + pnt[i] = 0U; + ++i; } -#endif #endif } -int32_t crypto_memcmp(const void *p1, const void *p2, size_t length) +int32_t crypto_memcmp(const uint8_t *p1, const uint8_t *p2, size_t length) { #ifndef VANILLA_NACL return sodium_memcmp(p1, p2, length); #else - const volatile unsigned char *volatile b1 = - (const volatile unsigned char *volatile) p1; - const volatile unsigned char *volatile b2 = - (const volatile unsigned char *volatile) p2; + //!TOKSTYLE- + const volatile uint8_t *volatile b1 = p1; + const volatile uint8_t *volatile b2 = p2; + //!TOKSTYLE+ size_t i; - unsigned char d = (unsigned char) 0U; + uint8_t d = (uint8_t) 0U; - for (i = 0U; i < length; i++) { + for (i = 0U; i < length; ++i) { d |= b1[i] ^ b2[i]; } diff --git a/toxcore/crypto_core_test.cc b/toxcore/crypto_core_test.cc index ee3cb366..66569506 100644 --- a/toxcore/crypto_core_test.cc +++ b/toxcore/crypto_core_test.cc @@ -1,6 +1,7 @@ #include "crypto_core.h" #include +#include #include @@ -29,7 +30,7 @@ enum { CRYPTO_TEST_MEMCMP_EPS = 10, }; -clock_t memcmp_time(void *a, void *b, size_t len) { +clock_t memcmp_time(uint8_t const *a, uint8_t const *b, size_t len) { clock_t start = clock(); volatile int result = crypto_memcmp(a, b, len); (void)result; @@ -41,8 +42,8 @@ clock_t memcmp_time(void *a, void *b, size_t len) { * equal and non-equal arrays to reduce the influence of external effects * such as the machine being a little more busy 1 second later. */ -void memcmp_median(void *src, void *same, void *not_same, size_t len, clock_t *same_median, - clock_t *not_same_median) { +std::pair memcmp_median(uint8_t const *src, uint8_t const *same, + uint8_t const *not_same, size_t len) { clock_t same_results[CRYPTO_TEST_MEMCMP_ITERATIONS]; clock_t not_same_results[CRYPTO_TEST_MEMCMP_ITERATIONS]; @@ -52,9 +53,10 @@ void memcmp_median(void *src, void *same, void *not_same, size_t len, clock_t *s } std::sort(same_results, same_results + CRYPTO_TEST_MEMCMP_ITERATIONS); - *same_median = same_results[CRYPTO_TEST_MEMCMP_ITERATIONS / 2]; + clock_t const same_median = same_results[CRYPTO_TEST_MEMCMP_ITERATIONS / 2]; std::sort(not_same_results, not_same_results + CRYPTO_TEST_MEMCMP_ITERATIONS); - *not_same_median = not_same_results[CRYPTO_TEST_MEMCMP_ITERATIONS / 2]; + clock_t const not_same_median = not_same_results[CRYPTO_TEST_MEMCMP_ITERATIONS / 2]; + return {same_median, not_same_median}; } /** @@ -63,32 +65,28 @@ void memcmp_median(void *src, void *same, void *not_same, size_t len, clock_t *s */ TEST(CryptoCore, MemcmpTimingIsDataIndependent) { // A random piece of memory. - auto *src = new uint8_t[CRYPTO_TEST_MEMCMP_SIZE]; - random_bytes(src, CRYPTO_TEST_MEMCMP_SIZE); + std::vector src(CRYPTO_TEST_MEMCMP_SIZE); + random_bytes(src.data(), CRYPTO_TEST_MEMCMP_SIZE); // A separate piece of memory containing the same data. - auto *same = new uint8_t[CRYPTO_TEST_MEMCMP_SIZE]; - memcpy(same, src, CRYPTO_TEST_MEMCMP_SIZE); + std::vector same = src; // Another piece of memory containing different data. - auto *not_same = new uint8_t[CRYPTO_TEST_MEMCMP_SIZE]; - random_bytes(not_same, CRYPTO_TEST_MEMCMP_SIZE); + std::vector not_same(CRYPTO_TEST_MEMCMP_SIZE); + random_bytes(not_same.data(), CRYPTO_TEST_MEMCMP_SIZE); - clock_t same_median; - clock_t not_same_median; - memcmp_median(src, same, not_same, CRYPTO_TEST_MEMCMP_SIZE, &same_median, ¬_same_median); - - delete[] not_same; - delete[] same; - delete[] src; + // Once we have C++17: + // auto const [same_median, not_same_median] = + auto const result = + memcmp_median(src.data(), same.data(), not_same.data(), CRYPTO_TEST_MEMCMP_SIZE); clock_t const delta = - same_median > not_same_median ? same_median - not_same_median : not_same_median - same_median; + std::max(result.first, result.second) - std::min(result.first, result.second); EXPECT_LT(delta, CRYPTO_TEST_MEMCMP_EPS) << "Delta time is too long (" << delta << " >= " << CRYPTO_TEST_MEMCMP_EPS << ")\n" - << "Time of the same data comparison: " << same_median << " clocks\n" - << "Time of the different data comparison: " << not_same_median << " clocks"; + << "Time of the same data comparison: " << result.first << " clocks\n" + << "Time of the different data comparison: " << result.second << " clocks"; } } // namespace diff --git a/toxcore/ping_array.api.h b/toxcore/ping_array.api.h index 81ede8fa..d36e56dc 100644 --- a/toxcore/ping_array.api.h +++ b/toxcore/ping_array.api.h @@ -20,7 +20,7 @@ extern "C" { class mono_Time { struct this; } -class ping_Array { +class ping { class array { struct this; @@ -55,7 +55,7 @@ uint64_t add(const mono_Time::this *mono_time, const uint8_t *data, uint32_t len */ int32_t check(const mono_Time::this *mono_time, uint8_t[length] data, uint64_t ping_id); -} +} } %{ #ifdef __cplusplus diff --git a/toxcore/ping_array.h b/toxcore/ping_array.h index 589573c8..70c517de 100644 --- a/toxcore/ping_array.h +++ b/toxcore/ping_array.h @@ -39,14 +39,14 @@ struct Ping_Array *ping_array_new(uint32_t size, uint32_t timeout); /** * Free all the allocated memory in a Ping_Array. */ -void ping_array_kill(struct Ping_Array *_array); +void ping_array_kill(struct Ping_Array *array); /** * Add a data with length to the Ping_Array list and return a ping_id. * * @return ping_id on success, 0 on failure. */ -uint64_t ping_array_add(struct Ping_Array *_array, const struct Mono_Time *mono_time, const uint8_t *data, +uint64_t ping_array_add(struct Ping_Array *array, const struct Mono_Time *mono_time, const uint8_t *data, uint32_t length); /** @@ -56,7 +56,7 @@ uint64_t ping_array_add(struct Ping_Array *_array, const struct Mono_Time *mono_ * * @return length of data copied on success, -1 on failure. */ -int32_t ping_array_check(struct Ping_Array *_array, const struct Mono_Time *mono_time, uint8_t *data, size_t length, +int32_t ping_array_check(struct Ping_Array *array, const struct Mono_Time *mono_time, uint8_t *data, size_t length, uint64_t ping_id); #ifdef __cplusplus diff --git a/toxcore/tox.api.h b/toxcore/tox.api.h index fcd43f7a..5e42b866 100644 --- a/toxcore/tox.api.h +++ b/toxcore/tox.api.h @@ -14,6 +14,8 @@ #include #include +//!TOKSTYLE- + #ifdef __cplusplus extern "C" { #endif @@ -2854,5 +2856,7 @@ typedef TOX_CONNECTION Tox_Connection; typedef TOX_FILE_CONTROL Tox_File_Control; typedef TOX_CONFERENCE_TYPE Tox_Conference_Type; +//!TOKSTYLE+ + #endif // C_TOXCORE_TOXCORE_TOX_H %} diff --git a/toxcore/tox.h b/toxcore/tox.h index dc873f3b..be6f6d34 100644 --- a/toxcore/tox.h +++ b/toxcore/tox.h @@ -13,6 +13,8 @@ #include #include +//!TOKSTYLE- + #ifdef __cplusplus extern "C" { #endif @@ -3251,4 +3253,6 @@ typedef TOX_CONNECTION Tox_Connection; typedef TOX_FILE_CONTROL Tox_File_Control; typedef TOX_CONFERENCE_TYPE Tox_Conference_Type; +//!TOKSTYLE+ + #endif // C_TOXCORE_TOXCORE_TOX_H diff --git a/toxcore/tox_api.c b/toxcore/tox_api.c index 8503f237..63b4bea3 100644 --- a/toxcore/tox_api.c +++ b/toxcore/tox_api.c @@ -7,6 +7,7 @@ #define SET_ERROR_PARAMETER(param, x) do { if (param) { *param = x; } } while (0) +//!TOKSTYLE- #define CONST_FUNCTION(lowercase, uppercase) \ uint32_t tox_##lowercase(void) \ @@ -60,6 +61,8 @@ ACCESSORS(void *, log_, user_data) ACCESSORS(bool,, local_discovery_enabled) ACCESSORS(bool,, experimental_thread_safety) +//!TOKSTYLE+ + const uint8_t *tox_options_get_savedata_data(const struct Tox_Options *options) { return options->savedata_data; diff --git a/toxencryptsave/crypto_pwhash_scryptsalsa208sha256/pwhash_scryptsalsa208sha256.c b/toxencryptsave/crypto_pwhash_scryptsalsa208sha256/pwhash_scryptsalsa208sha256.c index e2de3e5f..3aab156f 100644 --- a/toxencryptsave/crypto_pwhash_scryptsalsa208sha256/pwhash_scryptsalsa208sha256.c +++ b/toxencryptsave/crypto_pwhash_scryptsalsa208sha256/pwhash_scryptsalsa208sha256.c @@ -201,7 +201,7 @@ crypto_pwhash_scryptsalsa208sha256_str_verify(const char str[crypto_pwhash_scryp return -1; } escrypt_free_local(&escrypt_local); - ret = crypto_memcmp(wanted, str, sizeof wanted); + ret = crypto_memcmp((const uint8_t *) wanted, (const uint8_t *) str, sizeof wanted); crypto_memzero(wanted, sizeof wanted); return ret; diff --git a/toxencryptsave/defines.h b/toxencryptsave/defines.h index 0bc1d9ed..8a490344 100644 --- a/toxencryptsave/defines.h +++ b/toxencryptsave/defines.h @@ -1,7 +1,7 @@ #ifndef C_TOXCORE_TOXENCRYPTSAVE_DEFINES_H #define C_TOXCORE_TOXENCRYPTSAVE_DEFINES_H -#define TOX_ENC_SAVE_MAGIC_NUMBER "toxEsave" +#define TOX_ENC_SAVE_MAGIC_NUMBER ((const uint8_t *)"toxEsave") #define TOX_ENC_SAVE_MAGIC_LENGTH 8 #endif -- cgit v1.2.3