Skip to content

Commit

Permalink
cleanup: Reduce name shadowing; remove ptr-to-bool conversions.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
iphydf committed Feb 23, 2022
1 parent 70d75ae commit 17c8f50
Show file tree
Hide file tree
Showing 13 changed files with 93 additions and 70 deletions.
2 changes: 1 addition & 1 deletion other/bootstrap_daemon/docker/tox-bootstrapd.sha256
Original file line number Diff line number Diff line change
@@ -1 +1 @@
30dec481315934168e3b8abf54fe4ed5814161230bd499e0f935a4df1922dd33 /usr/local/bin/tox-bootstrapd
d69f99a7aa2e9290024b1dea3443fbfe9bfbaba444db5ccace5df2d88f15300a /usr/local/bin/tox-bootstrapd
8 changes: 4 additions & 4 deletions toxcore/DHT.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}

Expand Down
4 changes: 2 additions & 2 deletions toxcore/LAN_discovery.c
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand All @@ -108,7 +108,7 @@ static Broadcast_Info *fetch_broadcast_info(uint16_t port)
}
}

if (pAdapterInfo) {
if (pAdapterInfo != nullptr) {
free(pAdapterInfo);
}

Expand Down
10 changes: 5 additions & 5 deletions toxcore/Messenger.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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;
}

Expand Down
3 changes: 1 addition & 2 deletions toxcore/Messenger.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 */

Expand Down
2 changes: 1 addition & 1 deletion toxcore/TCP_common.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion toxcore/logger.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
57 changes: 34 additions & 23 deletions toxcore/mono_time.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand All @@ -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
Expand All @@ -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) {
Expand All @@ -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;

Expand All @@ -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
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);
#endif
time = 1000ULL * clock_mono.tv_sec + (clock_mono.tv_nsec / 1000000ULL);
#endif
return time;
return timespec_to_u64(clock_mono);
}
#endif // !__APPLE__
#endif // !OS_WIN32


#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
Expand Down Expand Up @@ -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
Expand All @@ -179,34 +191,33 @@ 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);
}

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
}

Expand Down Expand Up @@ -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;
}
43 changes: 28 additions & 15 deletions toxcore/network.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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
char tmp[256];

errno = 0;

#ifdef _GNU_SOURCE
const char *retstr = strerror_r(error, tmp, sizeof(tmp));
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, sizeof(tmp), "error %d (strerror_r failed with errno %d)", error, errno);
snprintf(tmp, tmp_size, "error %d (strerror_r failed with errno %d)", error, errno);
}

return retstr;
}
#else
const int fmt_error = strerror_r(error, tmp, sizeof(tmp));
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, sizeof(tmp), "error %d (strerror_r failed with error %d, errno %d)", error, fmt_error, errno);
snprintf(tmp, tmp_size, "error %d (strerror_r failed with error %d, errno %d)", error, fmt_error, errno);
}

const char *retstr = tmp;
return tmp;
}
#endif
char *net_new_strerror(int error)
{
char tmp[256];

errno = 0;

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) {
Expand All @@ -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)
{
Expand Down
Loading

0 comments on commit 17c8f50

Please sign in to comment.