From 17c8f50f44ae926c7969abd201f864479e1be8d2 Mon Sep 17 00:00:00 2001 From: iphydf Date: Wed, 23 Feb 2022 11:11:35 +0000 Subject: [PATCH] cleanup: Reduce name shadowing; remove ptr-to-bool conversions. 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. --- .../docker/tox-bootstrapd.sha256 | 2 +- toxcore/DHT.c | 8 +-- toxcore/LAN_discovery.c | 4 +- toxcore/Messenger.c | 10 ++-- toxcore/Messenger.h | 3 +- toxcore/TCP_common.c | 2 +- toxcore/logger.c | 2 +- toxcore/mono_time.c | 59 +++++++++++-------- toxcore/network.c | 59 +++++++++++-------- toxcore/onion_announce.c | 22 +++---- toxcore/onion_announce.h | 2 +- toxcore/onion_client.c | 6 +- toxcore/tox.c | 2 +- 13 files changed, 102 insertions(+), 79 deletions(-) diff --git a/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 b/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 index 761bd1ef..a4ea6979 100644 --- a/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 +++ b/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 @@ -1 +1 @@ -30dec481315934168e3b8abf54fe4ed5814161230bd499e0f935a4df1922dd33 /usr/local/bin/tox-bootstrapd +d69f99a7aa2e9290024b1dea3443fbfe9bfbaba444db5ccace5df2d88f15300a /usr/local/bin/tox-bootstrapd diff --git a/toxcore/DHT.c b/toxcore/DHT.c index 050ffd74..8a40e3ee 100644 --- a/toxcore/DHT.c +++ b/toxcore/DHT.c @@ -340,7 +340,7 @@ int create_request(const uint8_t *send_public_key, const uint8_t *send_secret_ke memcpy(temp + 1, data, length); temp[0] = request_id; const int len = encrypt_data(recv_public_key, send_secret_key, nonce, temp, length + 1, - CRYPTO_SIZE + packet); + packet + CRYPTO_SIZE); if (len == -1) { crypto_memzero(temp, MAX_CRYPTO_REQUEST_SIZE); @@ -942,13 +942,13 @@ static int dht_cmp_entry(const void *a, const void *b) return 1; } - const int close = id_closest(cmp_public_key, entry1.public_key, entry2.public_key); + const int closest = id_closest(cmp_public_key, entry1.public_key, entry2.public_key); - if (close == 1) { + if (closest == 1) { return 1; } - if (close == 2) { + if (closest == 2) { return -1; } diff --git a/toxcore/LAN_discovery.c b/toxcore/LAN_discovery.c index 01c2adb2..55c36f87 100644 --- a/toxcore/LAN_discovery.c +++ b/toxcore/LAN_discovery.c @@ -82,7 +82,7 @@ static Broadcast_Info *fetch_broadcast_info(uint16_t port) if (ret == NO_ERROR) { IP_ADAPTER_INFO *pAdapter = pAdapterInfo; - while (pAdapter) { + while (pAdapter != nullptr) { IP gateway = {0}; IP subnet_mask = {0}; @@ -108,7 +108,7 @@ static Broadcast_Info *fetch_broadcast_info(uint16_t port) } } - if (pAdapterInfo) { + if (pAdapterInfo != nullptr) { free(pAdapterInfo); } diff --git a/toxcore/Messenger.c b/toxcore/Messenger.c index 69d26dda..fb84e21a 100644 --- a/toxcore/Messenger.c +++ b/toxcore/Messenger.c @@ -317,7 +317,7 @@ static int clear_receipts(Messenger *m, int32_t friendnumber) struct Receipts *receipts = m->friendlist[friendnumber].receipts_start; - while (receipts) { + while (receipts != nullptr) { struct Receipts *temp_r = receipts->next; free(receipts); receipts = temp_r; @@ -378,7 +378,7 @@ static int do_receipts(Messenger *m, int32_t friendnumber, void *userdata) struct Receipts *receipts = m->friendlist[friendnumber].receipts_start; - while (receipts) { + while (receipts != nullptr) { if (friend_received_packet(m, friendnumber, receipts->packet_num) == -1) { break; } @@ -1422,8 +1422,8 @@ static int64_t send_file_data_packet(const Messenger *m, int32_t friendnumber, u * return -6 if packet queue full. * return -7 if wrong position. */ -int file_data(const Messenger *m, int32_t friendnumber, uint32_t filenumber, uint64_t position, const uint8_t *data, - uint16_t length) +int send_file_data(const Messenger *m, int32_t friendnumber, uint32_t filenumber, uint64_t position, + const uint8_t *data, uint16_t length) { assert(length == 0 || data != nullptr); @@ -1542,7 +1542,7 @@ static bool do_all_filetransfers(Messenger *m, int32_t friendnumber, void *userd } else if (ft->status == FILESTATUS_TRANSFERRING && ft->paused == FILE_PAUSE_NOT) { if (ft->size == 0) { /* Send 0 data to friend if file is 0 length. */ - file_data(m, friendnumber, i, 0, nullptr, 0); + send_file_data(m, friendnumber, i, 0, nullptr, 0); continue; } diff --git a/toxcore/Messenger.h b/toxcore/Messenger.h index 13aede3d..eff3a6d4 100644 --- a/toxcore/Messenger.h +++ b/toxcore/Messenger.h @@ -665,8 +665,7 @@ int file_seek(const Messenger *m, int32_t friendnumber, uint32_t filenumber, uin * return -7 if wrong position. */ non_null(1) nullable(5) -int file_data(const Messenger *m, int32_t friendnumber, uint32_t filenumber, uint64_t position, const uint8_t *data, - uint16_t length); +int send_file_data(const Messenger *m, int32_t friendnumber, uint32_t filenumber, uint64_t position, const uint8_t *data, uint16_t length); /*** A/V related */ diff --git a/toxcore/TCP_common.c b/toxcore/TCP_common.c index c19bd3d9..a91d4f7b 100644 --- a/toxcore/TCP_common.c +++ b/toxcore/TCP_common.c @@ -56,7 +56,7 @@ int send_pending_data(const Logger *logger, TCP_Connection *con) TCP_Priority_List *p = con->priority_queue_start; - while (p) { + while (p != nullptr) { const uint16_t left = p->size - p->sent; const int len = net_send(logger, con->sock, p->data + p->sent, left, &con->ip_port); diff --git a/toxcore/logger.c b/toxcore/logger.c index e1d0ee16..cc5e56dd 100644 --- a/toxcore/logger.c +++ b/toxcore/logger.c @@ -105,7 +105,7 @@ void logger_write(const Logger *log, Logger_Level level, const char *file, int l // On Windows, the path separator *may* be a backslash, so we look for that // one too. const char *windows_filename = strrchr(file, '\\'); - file = windows_filename ? windows_filename + 1 : file; + file = windows_filename != nullptr ? windows_filename + 1 : file; #endif // Format message diff --git a/toxcore/mono_time.c b/toxcore/mono_time.c index a27a22ce..7673b2d4 100644 --- a/toxcore/mono_time.c +++ b/toxcore/mono_time.c @@ -40,7 +40,7 @@ /** don't call into system billions of times for no reason */ struct Mono_Time { - uint64_t time; + uint64_t cur_time; uint64_t base_time; #ifdef OS_WIN32 /* protect `last_clock_update` and `last_clock_mono` from concurrent access */ @@ -56,11 +56,10 @@ struct Mono_Time { void *user_data; }; +#ifdef OS_WIN32 non_null(1) nullable(2) static uint64_t current_time_monotonic_default(Mono_Time *mono_time, void *user_data) { - uint64_t time = 0; -#ifdef OS_WIN32 /* Must hold mono_time->last_clock_lock here */ /* GetTickCount provides only a 32 bit counter, but we can't use @@ -70,7 +69,7 @@ static uint64_t current_time_monotonic_default(Mono_Time *mono_time, void *user_ uint32_t ticks = GetTickCount(); /* the higher 32 bits count the number of wrap arounds */ - uint64_t old_ovf = mono_time->time & ~((uint64_t)UINT32_MAX); + uint64_t old_ovf = mono_time->cur_time & ~((uint64_t)UINT32_MAX); /* Check if time has decreased because of 32 bit wrap from GetTickCount() */ if (ticks < mono_time->last_clock_mono) { @@ -84,10 +83,18 @@ static uint64_t current_time_monotonic_default(Mono_Time *mono_time, void *user_ } /* splice the low and high bits back together */ - time = old_ovf + ticks; -#else + return old_ovf + ticks; +} +#else // !OS_WIN32 +static uint64_t timespec_to_u64(struct timespec clock_mono) +{ + return 1000ULL * clock_mono.tv_sec + (clock_mono.tv_nsec / 1000000ULL); +} +#ifdef __APPLE__ +non_null(1) nullable(2) +static uint64_t current_time_monotonic_default(Mono_Time *mono_time, void *user_data) +{ struct timespec clock_mono; -#if defined(__APPLE__) clock_serv_t muhclock; mach_timespec_t machtime; @@ -97,13 +104,18 @@ static uint64_t current_time_monotonic_default(Mono_Time *mono_time, void *user_ clock_mono.tv_sec = machtime.tv_sec; clock_mono.tv_nsec = machtime.tv_nsec; -#else - clock_gettime(CLOCK_MONOTONIC, &clock_mono); -#endif - time = 1000ULL * clock_mono.tv_sec + (clock_mono.tv_nsec / 1000000ULL); -#endif - return time; + return timespec_to_u64(clock_mono); } +#else // !__APPLE__ +non_null(1) nullable(2) +static uint64_t current_time_monotonic_default(Mono_Time *mono_time, void *user_data) +{ + struct timespec clock_mono; + clock_gettime(CLOCK_MONOTONIC, &clock_mono); + return timespec_to_u64(clock_mono); +} +#endif // !__APPLE__ +#endif // !OS_WIN32 #ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION @@ -155,7 +167,7 @@ Mono_Time *mono_time_new(void) #endif - mono_time->time = 0; + mono_time->cur_time = 0; #ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION mono_time->base_time = 0; // Maximum reproducibility #else @@ -179,20 +191,20 @@ void mono_time_free(Mono_Time *mono_time) void mono_time_update(Mono_Time *mono_time) { - uint64_t time = 0; + uint64_t cur_time = 0; #ifdef OS_WIN32 /* we actually want to update the overflow state of mono_time here */ pthread_mutex_lock(&mono_time->last_clock_lock); mono_time->last_clock_update = true; #endif - time = mono_time->current_time_callback(mono_time, mono_time->user_data) / 1000ULL; - time += mono_time->base_time; + cur_time = mono_time->current_time_callback(mono_time, mono_time->user_data) / 1000ULL; + cur_time += mono_time->base_time; #ifdef OS_WIN32 pthread_mutex_unlock(&mono_time->last_clock_lock); #endif pthread_rwlock_wrlock(mono_time->time_update_lock); - mono_time->time = time; + mono_time->cur_time = cur_time; pthread_rwlock_unlock(mono_time->time_update_lock); } @@ -200,13 +212,12 @@ uint64_t mono_time_get(const Mono_Time *mono_time) { #ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION // Fuzzing is only single thread for now, no locking needed */ - return mono_time->time; + return mono_time->cur_time; #else - uint64_t time = 0; pthread_rwlock_rdlock(mono_time->time_update_lock); - time = mono_time->time; + const uint64_t cur_time = mono_time->cur_time; pthread_rwlock_unlock(mono_time->time_update_lock); - return time; + return cur_time; #endif } @@ -239,9 +250,9 @@ uint64_t current_time_monotonic(Mono_Time *mono_time) * but must protect against other threads */ pthread_mutex_lock(&mono_time->last_clock_lock); #endif - uint64_t time = mono_time->current_time_callback(mono_time, mono_time->user_data); + const uint64_t cur_time = mono_time->current_time_callback(mono_time, mono_time->user_data); #ifdef OS_WIN32 pthread_mutex_unlock(&mono_time->last_clock_lock); #endif - return time; + return cur_time; } diff --git a/toxcore/network.c b/toxcore/network.c index 496402c6..4b7260f8 100644 --- a/toxcore/network.c +++ b/toxcore/network.c @@ -830,9 +830,9 @@ Networking_Core *new_networking_ex(const Logger *log, const IP *ip, uint16_t por } else if (port_from != 0 && port_to == 0) { port_to = port_from; } else if (port_from > port_to) { - uint16_t temp = port_from; + uint16_t temp_port = port_from; port_from = port_to; - port_to = temp; + port_to = temp_port; } if (error != nullptr) { @@ -1262,7 +1262,7 @@ bool ip_parse_addr(const IP *ip, char *address, size_t length) if (net_family_is_ipv4(ip->family)) { struct in_addr addr; static_assert(sizeof(addr) == sizeof(ip->ip.v4.uint32), - "assumption does not hold: in_addr should be 4 bytes"); + "assumption does not hold: in_addr should be 4 bytes"); assert(make_family(ip->family) == AF_INET); fill_addr4(&ip->ip.v4, &addr); return inet_ntop4(&addr, address, length) != nullptr; @@ -1271,7 +1271,7 @@ bool ip_parse_addr(const IP *ip, char *address, size_t length) if (net_family_is_ipv6(ip->family)) { struct in6_addr addr; static_assert(sizeof(addr) == sizeof(ip->ip.v6.uint8), - "assumption does not hold: in6_addr should be 16 bytes"); + "assumption does not hold: in6_addr should be 16 bytes"); assert(make_family(ip->family) == AF_INET6); fill_addr6(&ip->ip.v6, &addr); return inet_ntop6(&addr, address, length) != nullptr; @@ -1739,9 +1739,9 @@ int net_error(void) #endif } +#if defined(_WIN32) || defined(__WIN32__) || defined(WIN32) char *net_new_strerror(int error) { -#if defined(_WIN32) || defined(__WIN32__) || defined(WIN32) char *str = nullptr; // Windows API is weird. The 5th function arg is of char* type, but we // have to pass char** so that it could assign new memory block to our @@ -1753,29 +1753,42 @@ char *net_new_strerror(int error) FormatMessageA(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS, nullptr, error, 0, (char *)&str, 0, nullptr); return str; +} #else +#ifdef _GNU_SOURCE +non_null() +static const char *net_strerror_r(int error, char *tmp, size_t tmp_size) +{ + const char *retstr = strerror_r(error, tmp, tmp_size); + + if (errno != 0) { + snprintf(tmp, tmp_size, "error %d (strerror_r failed with errno %d)", error, errno); + } + + return retstr; +} +#else +non_null() +static const char *net_strerror_r(int error, char *tmp, size_t tmp_size) +{ + const int fmt_error = strerror_r(error, tmp, tmp_size); + + if (fmt_error != 0) { + snprintf(tmp, tmp_size, "error %d (strerror_r failed with error %d, errno %d)", error, fmt_error, errno); + } + + return tmp; +} +#endif +char *net_new_strerror(int error) +{ char tmp[256]; errno = 0; -#ifdef _GNU_SOURCE - const char *retstr = strerror_r(error, tmp, sizeof(tmp)); - - if (errno != 0) { - snprintf(tmp, sizeof(tmp), "error %d (strerror_r failed with errno %d)", error, errno); - } - -#else - const int fmt_error = strerror_r(error, tmp, sizeof(tmp)); - - if (fmt_error != 0) { - snprintf(tmp, sizeof(tmp), "error %d (strerror_r failed with error %d, errno %d)", error, fmt_error, errno); - } - - const char *retstr = tmp; -#endif - + const char *retstr = net_strerror_r(error, tmp, sizeof(tmp)); const size_t retstr_len = strlen(retstr); + char *str = (char *)malloc(retstr_len + 1); if (str == nullptr) { @@ -1785,8 +1798,8 @@ char *net_new_strerror(int error) memcpy(str, retstr, retstr_len + 1); return str; -#endif } +#endif void net_kill_strerror(char *strerror) { diff --git a/toxcore/onion_announce.c b/toxcore/onion_announce.c index 26e76cf5..1d51f174 100644 --- a/toxcore/onion_announce.c +++ b/toxcore/onion_announce.c @@ -56,9 +56,9 @@ uint8_t *onion_announce_entry_public_key(Onion_Announce *onion_a, uint32_t entry return onion_a->entries[entry].public_key; } -void onion_announce_entry_set_time(Onion_Announce *onion_a, uint32_t entry, uint64_t time) +void onion_announce_entry_set_time(Onion_Announce *onion_a, uint32_t entry, uint64_t cur_time) { - onion_a->entries[entry].time = time; + onion_a->entries[entry].time = cur_time; } /** Create an onion announce request packet in packet of max_packet_length (recommended size ONION_ANNOUNCE_REQUEST_SIZE). @@ -231,15 +231,15 @@ int send_data_request(const Networking_Core *net, const Onion_Path *path, const /** Generate a ping_id and put it in ping_id */ non_null() -static void generate_ping_id(const Onion_Announce *onion_a, uint64_t time, const uint8_t *public_key, +static void generate_ping_id(const Onion_Announce *onion_a, uint64_t ping_time, const uint8_t *public_key, const IP_Port *ret_ip_port, uint8_t *ping_id) { - time /= PING_ID_TIMEOUT; - uint8_t data[CRYPTO_SYMMETRIC_KEY_SIZE + sizeof(time) + CRYPTO_PUBLIC_KEY_SIZE + sizeof(IP_Port)]; + ping_time /= PING_ID_TIMEOUT; + uint8_t data[CRYPTO_SYMMETRIC_KEY_SIZE + sizeof(ping_time) + CRYPTO_PUBLIC_KEY_SIZE + sizeof(IP_Port)]; memcpy(data, onion_a->secret_bytes, CRYPTO_SYMMETRIC_KEY_SIZE); - memcpy(data + CRYPTO_SYMMETRIC_KEY_SIZE, &time, sizeof(time)); - memcpy(data + CRYPTO_SYMMETRIC_KEY_SIZE + sizeof(time), public_key, CRYPTO_PUBLIC_KEY_SIZE); - memcpy(data + CRYPTO_SYMMETRIC_KEY_SIZE + sizeof(time) + CRYPTO_PUBLIC_KEY_SIZE, ret_ip_port, sizeof(IP_Port)); + memcpy(data + CRYPTO_SYMMETRIC_KEY_SIZE, &ping_time, sizeof(ping_time)); + memcpy(data + CRYPTO_SYMMETRIC_KEY_SIZE + sizeof(ping_time), public_key, CRYPTO_PUBLIC_KEY_SIZE); + memcpy(data + CRYPTO_SYMMETRIC_KEY_SIZE + sizeof(ping_time) + CRYPTO_PUBLIC_KEY_SIZE, ret_ip_port, sizeof(IP_Port)); crypto_sha256(ping_id, data, sizeof(data)); } @@ -291,13 +291,13 @@ static int cmp_entry(const void *a, const void *b) return 1; } - const int close = id_closest(cmp_public_key, entry1.public_key, entry2.public_key); + const int closest = id_closest(cmp_public_key, entry1.public_key, entry2.public_key); - if (close == 1) { + if (closest == 1) { return 1; } - if (close == 2) { + if (closest == 2) { return -1; } diff --git a/toxcore/onion_announce.h b/toxcore/onion_announce.h index 9b9bf238..c9cc1e79 100644 --- a/toxcore/onion_announce.h +++ b/toxcore/onion_announce.h @@ -34,7 +34,7 @@ typedef struct Onion_Announce Onion_Announce; non_null() uint8_t *onion_announce_entry_public_key(Onion_Announce *onion_a, uint32_t entry); non_null() -void onion_announce_entry_set_time(Onion_Announce *onion_a, uint32_t entry, uint64_t time); +void onion_announce_entry_set_time(Onion_Announce *onion_a, uint32_t entry, uint64_t cur_time); /** Create an onion announce request packet in packet of max_packet_length (recommended size ONION_ANNOUNCE_REQUEST_SIZE). * diff --git a/toxcore/onion_client.c b/toxcore/onion_client.c index a4f62eab..5893322d 100644 --- a/toxcore/onion_client.c +++ b/toxcore/onion_client.c @@ -641,13 +641,13 @@ static int onion_client_cmp_entry(const void *a, const void *b) return 1; } - const int close = id_closest(cmp_public_key, entry1.public_key, entry2.public_key); + const int closest = id_closest(cmp_public_key, entry1.public_key, entry2.public_key); - if (close == 1) { + if (closest == 1) { return 1; } - if (close == 2) { + if (closest == 2) { return -1; } diff --git a/toxcore/tox.c b/toxcore/tox.c index a52287f1..14fce0af 100644 --- a/toxcore/tox.c +++ b/toxcore/tox.c @@ -1744,7 +1744,7 @@ bool tox_file_send_chunk(Tox *tox, uint32_t friend_number, uint32_t file_number, { assert(tox != nullptr); lock(tox); - const int ret = file_data(tox->m, friend_number, file_number, position, data, length); + const int ret = send_file_data(tox->m, friend_number, file_number, position, data, length); unlock(tox); if (ret == 0) {