Skip to content

Commit 083d8d4

Browse files
committed
Enable more compiler warnings
To ensure code quality and make life harder. Notes: - choosed to use enum or int (e.g. ares_status_t) - CURLINFO_SSL_VERIFYRESULT was wrong - https_fetch_ctx_cleanup's curl_result_code parameter - int options range checked in options.c
1 parent 866a916 commit 083d8d4

File tree

8 files changed

+46
-44
lines changed

8 files changed

+46
-44
lines changed

CMakeLists.txt

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,7 @@ if (NOT CMAKE_INSTALL_BINDIR)
2525
set(CMAKE_INSTALL_BINDIR bin)
2626
endif()
2727

28-
# TODO: add flags: -Wconversion -Wsign-conversion -Wfloat-conversion -Wcast-align -Wimplicit-fallthrough
29-
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -Wextra -Wpedantic -Wstrict-aliasing -Wformat=2 -Wunused -Wno-variadic-macros -Wnull-dereference -Wshadow")
28+
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -Wextra -Wpedantic -Wstrict-aliasing -Wformat=2 -Wunused -Wno-variadic-macros -Wnull-dereference -Wshadow -Wconversion -Wsign-conversion -Wfloat-conversion -Wimplicit-fallthrough")
3029
set(CMAKE_C_FLAGS_DEBUG "-gdwarf-4 -DDEBUG")
3130
set(CMAKE_C_FLAGS_RELEASE "-O2")
3231

@@ -97,7 +96,7 @@ if(USE_CLANG_TIDY)
9796
message(STATUS "clang-tidy not found.")
9897
else()
9998
message(STATUS "clang-tidy found: ${CLANG_TIDY_EXE}")
100-
set(DO_CLANG_TIDY "${CLANG_TIDY_EXE}" "-fix" "-fix-errors" "-checks=*,-cert-err34-c,-readability-identifier-length,-altera-unroll-loops,-bugprone-easily-swappable-parameters,-concurrency-mt-unsafe,-*magic-numbers,-hicpp-signed-bitwise,-readability-function-cognitive-complexity,-altera-id-dependent-backward-branch,-google-readability-todo,-misc-include-cleaner")
99+
set(DO_CLANG_TIDY "${CLANG_TIDY_EXE}" "-fix" "-fix-errors" "-checks=*,-cert-err34-c,-readability-identifier-length,-altera-unroll-loops,-bugprone-easily-swappable-parameters,-concurrency-mt-unsafe,-*magic-numbers,-hicpp-signed-bitwise,-readability-function-cognitive-complexity,-altera-id-dependent-backward-branch,-google-readability-todo,-misc-include-cleaner,-cast-align")
101100
endif()
102101
else()
103102
message(STATUS "Not using clang-tidy.")

src/dns_poller.c

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ static void sock_cb(struct ev_loop __attribute__((unused)) *loop,
1212
}
1313

1414
static struct ev_io * get_io_event(dns_poller_t *d, int sock) {
15-
for (int i = 0; i < d->io_events_count; i++) {
15+
for (unsigned i = 0; i < d->io_events_count; i++) {
1616
if (d->io_events[i].fd == sock) {
1717
return &d->io_events[i];
1818
}
@@ -35,7 +35,7 @@ static void sock_state_cb(void *data, int fd, int read, int write) {
3535
// reserve and start new event on unused slot
3636
io_event_ptr = get_io_event(d, 0);
3737
if (!io_event_ptr) {
38-
FLOG("c-ares needed more IO event handler, than the number of provided nameservers: %d", d->io_events_count);
38+
FLOG("c-ares needed more IO event handler, than the number of provided nameservers: %u", d->io_events_count);
3939
}
4040
DLOG("Reserved new io event: %p", io_event_ptr);
4141
// NOLINTNEXTLINE(clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling)
@@ -52,7 +52,7 @@ static char *get_addr_listing(char** addr_list, const int af) {
5252
}
5353
for (int i = 0; addr_list[i]; i++) {
5454
const char *res = ares_inet_ntop(af, addr_list[i], pos,
55-
list + POLLER_ADDR_LIST_SIZE - 1 - pos);
55+
(ares_socklen_t)(list + POLLER_ADDR_LIST_SIZE - 1 - pos));
5656
if (res != NULL) {
5757
pos += strlen(pos);
5858
*pos = ',';
@@ -97,8 +97,8 @@ static ev_tstamp get_timeout(dns_poller_t *d)
9797
struct timeval tv;
9898
struct timeval *tvp = ares_timeout(d->ares, &max_tv, &tv);
9999
// NOLINTNEXTLINE(bugprone-narrowing-conversions,cppcoreguidelines-narrowing-conversions)
100-
ev_tstamp after = tvp->tv_sec + tvp->tv_usec * 1e-6;
101-
return after ? after : 0.1;
100+
ev_tstamp after = (double)tvp->tv_sec + (double)tvp->tv_usec * 1e-6;
101+
return after > 0.1 ? after : 0.1;
102102
}
103103

104104
static void timer_cb(struct ev_loop __attribute__((unused)) *loop,
@@ -170,8 +170,8 @@ void dns_poller_init(dns_poller_t *d, struct ev_loop *loop,
170170
d->timer.data = d;
171171
ev_timer_start(d->loop, &d->timer);
172172

173-
int nameservers = 1;
174-
for (int i = 0; bootstrap_dns[i]; i++) {
173+
unsigned nameservers = 1;
174+
for (unsigned i = 0; bootstrap_dns[i]; i++) {
175175
if (bootstrap_dns[i] == ',') {
176176
nameservers++;
177177
}
@@ -181,7 +181,7 @@ void dns_poller_init(dns_poller_t *d, struct ev_loop *loop,
181181
if (!d->io_events) {
182182
FLOG("Out of mem");
183183
}
184-
for (int i = 0; i < nameservers; i++) {
184+
for (unsigned i = 0; i < nameservers; i++) {
185185
d->io_events[i].data = d;
186186
}
187187
d->io_events_count = nameservers;

src/dns_poller.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ typedef struct {
3030

3131
ev_timer timer;
3232
ev_io *io_events;
33-
int io_events_count;
33+
unsigned io_events_count;
3434
} dns_poller_t;
3535

3636
// Initializes c-ares and starts a timer for periodic DNS resolution on the

src/dns_server.c

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -57,13 +57,13 @@ static void watcher_cb(struct ev_loop __attribute__((unused)) *loop,
5757
return;
5858
}
5959

60-
char *dns_req = (char *)malloc(len); // To free buffer after https request is complete.
60+
char *dns_req = (char *)malloc((size_t)len); // To free buffer after https request is complete.
6161
if (dns_req == NULL) {
6262
FLOG("Out of mem");
6363
}
64-
memcpy(dns_req, tmp_buf, len); // NOLINT(clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling)
64+
memcpy(dns_req, tmp_buf, (size_t)len); // NOLINT(clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling)
6565

66-
d->cb(d, 0, d->cb_data, (struct sockaddr*)&tmp_raddr, dns_req, len);
66+
d->cb(d, 0, d->cb_data, (struct sockaddr*)&tmp_raddr, dns_req, (size_t)len);
6767
}
6868

6969
void dns_server_init(dns_server_t *d, struct ev_loop *loop,
@@ -85,7 +85,7 @@ static uint16_t get_edns_udp_size(const char *dns_req, const size_t dns_req_len)
8585
ares_dns_record_t *dnsrec = NULL;
8686
ares_status_t parse_status = ares_dns_parse((const unsigned char *)dns_req, dns_req_len, 0, &dnsrec);
8787
if (parse_status != ARES_SUCCESS) {
88-
WLOG("Failed to parse DNS request: %s", ares_strerror(parse_status));
88+
WLOG("Failed to parse DNS request: %s", ares_strerror((int)parse_status));
8989
return DNS_SIZE_LIMIT;
9090
}
9191
const uint16_t tx_id = ares_dns_record_get_id(dnsrec);
@@ -116,7 +116,7 @@ static void truncate_dns_response(char *buf, size_t *buflen, const uint16_t size
116116
ares_dns_record_t *dnsrec = NULL;
117117
ares_status_t status = ares_dns_parse((const unsigned char *)buf, *buflen, 0, &dnsrec);
118118
if (status != ARES_SUCCESS) {
119-
WLOG("Failed to parse DNS response: %s", ares_strerror(status));
119+
WLOG("Failed to parse DNS response: %s", ares_strerror((int)status));
120120
return;
121121
}
122122
const uint16_t tx_id = ares_dns_record_get_id(dnsrec);
@@ -127,13 +127,13 @@ static void truncate_dns_response(char *buf, size_t *buflen, const uint16_t size
127127
while (ares_dns_record_rr_cnt(dnsrec, ARES_SECTION_ADDITIONAL) > 0) {
128128
status = ares_dns_record_rr_del(dnsrec, ARES_SECTION_ADDITIONAL, 0);
129129
if (status != ARES_SUCCESS) {
130-
WLOG("%04hX: Could not remove additional record: %s", tx_id, ares_strerror(status));
130+
WLOG("%04hX: Could not remove additional record: %s", tx_id, ares_strerror((int)status));
131131
}
132132
}
133133
while (ares_dns_record_rr_cnt(dnsrec, ARES_SECTION_AUTHORITY) > 0) {
134134
status = ares_dns_record_rr_del(dnsrec, ARES_SECTION_AUTHORITY, 0);
135135
if (status != ARES_SUCCESS) {
136-
WLOG("%04hX: Could not remove authority record: %s", tx_id, ares_strerror(status));
136+
WLOG("%04hX: Could not remove authority record: %s", tx_id, ares_strerror((int)status));
137137
}
138138
}
139139

@@ -148,7 +148,7 @@ static void truncate_dns_response(char *buf, size_t *buflen, const uint16_t size
148148
for (uint8_t g = 0; g < UINT8_MAX; ++g) { // endless loop guard
149149
status = ares_dns_write(dnsrec, &new_resp, &new_resp_len);
150150
if (status != ARES_SUCCESS) {
151-
WLOG("%04hX: Failed to create truncated DNS response: %s", tx_id, ares_strerror(status));
151+
WLOG("%04hX: Failed to create truncated DNS response: %s", tx_id, ares_strerror((int)status));
152152
new_resp = NULL; // just to be sure
153153
break;
154154
}
@@ -168,7 +168,7 @@ static void truncate_dns_response(char *buf, size_t *buflen, const uint16_t size
168168
while (answers > answers_to_keep) {
169169
status = ares_dns_record_rr_del(dnsrec, ARES_SECTION_ANSWER, answers - 1);
170170
if (status != ARES_SUCCESS) {
171-
WLOG("%04hX: Could not remove answer record: %s", tx_id, ares_strerror(status));
171+
WLOG("%04hX: Could not remove answer record: %s", tx_id, ares_strerror((int)status));
172172
break;
173173
}
174174
--answers;

src/https_client.c

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ static int closesocket_callback(void __attribute__((unused)) *clientp, curl_sock
138138
return 0;
139139
}
140140

141-
static void https_log_data(enum LogSeverity level, struct https_fetch_ctx *ctx,
141+
static void https_log_data(int level, struct https_fetch_ctx *ctx,
142142
const char * prefix, char *ptr, size_t size)
143143
{
144144
const size_t width = 0x10;
@@ -156,14 +156,14 @@ static void https_log_data(enum LogSeverity level, struct https_fetch_ctx *ctx,
156156
for (size_t c = 0; c < width; c++) {
157157
if (i+c < size) {
158158
// NOLINTNEXTLINE(clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling)
159-
hex_off += snprintf(hex + hex_off, sizeof(hex) - hex_off,
160-
"%02x ", (unsigned char)ptr[i+c]);
159+
hex_off += (size_t)snprintf(hex + hex_off, sizeof(hex) - hex_off,
160+
"%02x ", (unsigned char)ptr[i+c]);
161161
// NOLINTNEXTLINE(clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling)
162-
str_off += snprintf(str + str_off, sizeof(str) - str_off,
163-
"%c", isprint(ptr[i+c]) ? ptr[i+c] : '.');
162+
str_off += (size_t)snprintf(str + str_off, sizeof(str) - str_off,
163+
"%c", isprint(ptr[i+c]) ? ptr[i+c] : '.');
164164
} else {
165165
// NOLINTNEXTLINE(clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling)
166-
hex_off += snprintf(hex + hex_off, sizeof(hex) - hex_off, " ");
166+
hex_off += (size_t)snprintf(hex + hex_off, sizeof(hex) - hex_off, " ");
167167
}
168168
}
169169

@@ -249,7 +249,7 @@ static void https_set_request_version(https_client_t *client,
249249
switch (client->opt->use_http_version) {
250250
case 1:
251251
http_version_int = CURL_HTTP_VERSION_1_1;
252-
// fallthrough
252+
__attribute__((fallthrough));
253253
case 2:
254254
break;
255255
case 3:
@@ -338,7 +338,7 @@ static void https_fetch_ctx_init(https_client_t *client,
338338

339339
static int https_fetch_ctx_process_response(https_client_t *client,
340340
struct https_fetch_ctx *ctx,
341-
int curl_result_code)
341+
CURLcode curl_result_code)
342342
{
343343
CURLcode res = 0;
344344
long long_resp = 0;
@@ -419,15 +419,15 @@ static int https_fetch_ctx_process_response(https_client_t *client,
419419
res = curl_easy_getinfo(ctx->curl, CURLINFO_SSL_VERIFYRESULT, &long_resp);
420420
if (res != CURLE_OK) {
421421
ELOG_REQ("CURLINFO_SSL_VERIFYRESULT: %s", curl_easy_strerror(res));
422-
} else if (long_resp != CURLE_OK) {
423-
WLOG_REQ("CURLINFO_SSL_VERIFYRESULT: %s", curl_easy_strerror(long_resp));
422+
} else if (long_resp != 0) {
423+
WLOG_REQ("CURLINFO_SSL_VERIFYRESULT: certificate verification failure %d", long_resp);
424424
}
425425

426426
res = curl_easy_getinfo(ctx->curl, CURLINFO_OS_ERRNO, &long_resp);
427427
if (res != CURLE_OK) {
428428
ELOG_REQ("CURLINFO_OS_ERRNO: %s", curl_easy_strerror(res));
429429
} else if (long_resp != 0) {
430-
WLOG_REQ("CURLINFO_OS_ERRNO: %d %s", long_resp, strerror(long_resp));
430+
WLOG_REQ("CURLINFO_OS_ERRNO: %d %s", long_resp, strerror((int)long_resp));
431431
if (long_resp == ENETUNREACH && !client->opt->ipv4) {
432432
// this can't be fixed here with option overwrite because of dns_poller
433433
WLOG("Try to run application with -4 argument!");
@@ -510,7 +510,7 @@ static void https_fetch_ctx_cleanup(https_client_t *client,
510510
if (curl_result_code < 0) {
511511
WLOG_REQ("Request was aborted");
512512
drop_reply = 1;
513-
} else if (https_fetch_ctx_process_response(client, ctx, curl_result_code) != 0) {
513+
} else if (https_fetch_ctx_process_response(client, ctx, (CURLcode)curl_result_code) != 0) {
514514
ILOG_REQ("Response was faulty, skipping DNS reply");
515515
drop_reply = 1;
516516
}
@@ -540,7 +540,7 @@ static void check_multi_info(https_client_t *c) {
540540
struct https_fetch_ctx *cur = c->fetches;
541541
while (cur) {
542542
if (cur->curl == msg->easy_handle) {
543-
https_fetch_ctx_cleanup(c, prev, cur, msg->data.result);
543+
https_fetch_ctx_cleanup(c, prev, cur, (int)msg->data.result);
544544
break;
545545
}
546546
prev = cur;
@@ -642,7 +642,7 @@ static int multi_timer_cb(CURLM __attribute__((unused)) *multi,
642642
ev_timer_stop(c->loop, &c->timer);
643643
if (timeout_ms >= 0) {
644644
// NOLINTNEXTLINE(clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling)
645-
ev_timer_init(&c->timer, timer_cb, timeout_ms / 1000.0, 0);
645+
ev_timer_init(&c->timer, timer_cb, (double)timeout_ms / 1000.0, 0);
646646
ev_timer_start(c->loop, &c->timer);
647647
}
648648
return 0;

src/logging.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ void _log(const char *file, int line, int severity, const char *fmt, ...) {
121121
if (chars < 0 || chars >= LOG_LINE_SIZE/2) {
122122
abort(); // must be impossible
123123
}
124-
buff_pos += chars;
124+
buff_pos += (uint32_t)chars;
125125

126126
va_list args;
127127
va_start(args, fmt);
@@ -132,7 +132,7 @@ void _log(const char *file, int line, int severity, const char *fmt, ...) {
132132
if (chars < 0) {
133133
abort(); // must be impossible
134134
}
135-
buff_pos += chars;
135+
buff_pos += (uint32_t)chars;
136136
if (buff_pos >= LOG_LINE_SIZE) {
137137
buff_pos = LOG_LINE_SIZE - 1;
138138
buff[buff_pos - 1] = '$'; // indicate truncation

src/main.c

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ static int addr_list_reduced(const char* full_list, const char* list) {
156156
while (pos < end) {
157157
char current[50];
158158
const char *comma = strchr(pos, ',');
159-
size_t ip_len = comma ? comma - pos : end - pos;
159+
size_t ip_len = (size_t)(comma ? comma - pos : end - pos);
160160
strncpy(current, pos, ip_len); // NOLINT(clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling)
161161
current[ip_len] = '\0';
162162

@@ -180,7 +180,10 @@ static void dns_poll_cb(const char* hostname, void *data,
180180
memset(buf, 0, sizeof(buf)); // NOLINT(clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling)
181181
if (strlen(hostname) > 254) { FLOG("Hostname too long."); }
182182
int ip_start = snprintf(buf, sizeof(buf) - 1, "%s:443:", hostname); // NOLINT(clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling)
183-
(void)snprintf(buf + ip_start, sizeof(buf) - 1 - ip_start, "%s", addr_list); // NOLINT(clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling)
183+
if (ip_start < 0) {
184+
abort(); // must be impossible
185+
}
186+
(void)snprintf(buf + ip_start, sizeof(buf) - 1 - (uint32_t)ip_start, "%s", addr_list); // NOLINT(clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling)
184187
if (app->resolv && app->resolv->data) {
185188
char * old_addr_list = strstr(app->resolv->data, ":443:");
186189
if (old_addr_list) {
@@ -272,7 +275,7 @@ int main(int argc, char *argv[]) {
272275
}
273276
case OPR_PARSING_ERROR:
274277
printf("Failed to parse options!\n");
275-
// fallthrough
278+
__attribute__((fallthrough));
276279
case OPR_OPTION_ERROR:
277280
printf("\n");
278281
options_show_usage(argc, argv);
@@ -281,7 +284,7 @@ int main(int argc, char *argv[]) {
281284
abort(); // must not happen
282285
}
283286

284-
logging_init(opt.logfd, opt.loglevel, opt.flight_recorder_size);
287+
logging_init(opt.logfd, opt.loglevel, (uint32_t)opt.flight_recorder_size);
285288

286289
ILOG("Version: %s", sw_version());
287290
ILOG("Built: " __DATE__ " " __TIME__);
@@ -323,7 +326,7 @@ int main(int argc, char *argv[]) {
323326
https_client_init(&https_client, &opt, (opt.stats_interval ? &stat : NULL), loop);
324327

325328
struct addrinfo *listen_addrinfo = get_listen_address(opt.listen_addr);
326-
((struct sockaddr_in*) listen_addrinfo->ai_addr)->sin_port = htons(opt.listen_port);
329+
((struct sockaddr_in*) listen_addrinfo->ai_addr)->sin_port = htons((uint16_t)opt.listen_port);
327330

328331
app_state_t app;
329332
app.https_client = &https_client;
@@ -338,7 +341,7 @@ int main(int argc, char *argv[]) {
338341

339342
dns_server_tcp_t * dns_server_tcp = NULL;
340343
if (opt.tcp_client_limit > 0) {
341-
dns_server_tcp = dns_server_tcp_create(loop, listen_addrinfo, dns_server_cb, &app, opt.tcp_client_limit);
344+
dns_server_tcp = dns_server_tcp_create(loop, listen_addrinfo, dns_server_cb, &app, (uint16_t)opt.tcp_client_limit);
342345
}
343346

344347
freeaddrinfo(listen_addrinfo);

src/options.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ struct Options {
5757
const char *ca_info;
5858

5959
// Number of logs to be kept by flight recorder
60-
uint32_t flight_recorder_size;
60+
int flight_recorder_size;
6161
} __attribute__((aligned(128)));
6262
typedef struct Options options_t;
6363

0 commit comments

Comments
 (0)