Fixes 17

src/dns_listener_tcp.c

@@ -1,6 +1,3 @@
-//NOLINTNEXTLINE(bugprone-reserved-identifier,cert-dcl37-c,cert-dcl51-cpp)
-#define _GNU_SOURCE  // needed for having accept4()
-
 #include <arpa/inet.h>
 #include <errno.h>
 #include <fcntl.h>
@@ -34,7 +31,8 @@
 enum {
   LISTEN_BACKLOG  =   5,
   IDLE_TIMEOUT_S  = 120,  // "two minutes" according to RFC1035 4.2.2
-  RESEND_DELAY_US = 500,  // 0.0005 sec
+  RESPONSE_SEND_ATTEMPTS = 50,   // 0.025 sec max wait
+  RESPONSE_SEND_DELAY_US = 500,  // 0.0005 sec
   TCP_DNS_MAX_PAYLOAD = UINT16_MAX - sizeof(uint16_t),  // Max after 2-byte length prefix
 };
 
@@ -95,17 +93,13 @@ static void remove_client(struct tcp_client_s * client) {
 
   close(client->sock);
 
-  // Save next pointer before freeing. Safe because this is single-threaded
-  // event loop - no callbacks can run during this function.
-  struct tcp_client_s *next = client->next;
-
   if (d->clients == client) {
-    d->clients = next;
+    d->clients = client->next;
   }
   else {
     for (struct tcp_client_s * cur = d->clients; cur != NULL; cur = cur->next) {
       if (cur->next == client) {
-        cur->next = next;
+        cur->next = client->next;
         break;
       }
     }
@@ -144,11 +138,11 @@ static void read_cb(struct ev_loop __attribute__((unused)) *loop,
   ssize_t len = recv(w->fd, buf, DNS_REQUEST_BUFFER_SIZE, 0);
   if (len <= 0) {
     if (len == 0 || errno == ECONNRESET) {
-      DLOG_CLIENT("Connection closed");
+      DLOG_CLIENT("TCP client closed connection");
     } else if (errno == EAGAIN || errno == EWOULDBLOCK) {
       return;
     } else {
-      WLOG_CLIENT("Read error: %s", strerror(errno));
+      WLOG_CLIENT("Read error: %s (%d), dropping client", strerror(errno), errno);
     }
     remove_client(client);
     return;
@@ -194,12 +188,13 @@ static void read_cb(struct ev_loop __attribute__((unused)) *loop,
   uint8_t request_received = 0;
   while (get_dns_request(client, &dns_req, &req_size)) {
     if (req_size < DNS_HEADER_LENGTH) {
-      WLOG_CLIENT("Malformed request received, too short: %u", req_size);
+      WLOG_CLIENT("Malformed request received, too short: %u, dropping client", req_size);
       free(dns_req);
       remove_client(client);
       return;
     }
 
+    DLOG_CLIENT("Requested %04hX", ntohs(*((uint16_t*)dns_req)));
     d->cb(d->cb_data, &d->base, (struct sockaddr*)&client->raddr, dns_req, req_size);
     request_received = 1;
   }
@@ -223,16 +218,27 @@ static void accept_cb(struct ev_loop __attribute__((unused)) *loop,
   struct sockaddr_storage client_addr;
   socklen_t client_addr_len = sizeof(client_addr);
 
-  int client_sock = accept(w->fd, (struct sockaddr *)&client_addr, &client_addr_len);
-  if (client_sock != -1) {
-    // Set non-blocking mode for macOS compatibility (Linux accept4 does this atomically)
-    int flags = fcntl(client_sock, F_GETFL, 0);
-    if (flags != -1) {
-      fcntl(client_sock, F_SETFL, flags | O_NONBLOCK);
+  // NOLINTNEXTLINE(android-cloexec-accept)
+  const int client_sock = accept(w->fd, (struct sockaddr *)&client_addr, &client_addr_len);
+  if (client_sock == -1) {
+    if (errno != EAGAIN && errno != EWOULDBLOCK) {
+      ELOG("Failed to accept TCP client: %s (%d)", strerror(errno), errno);
     }
+    return;
   }
-  if (client_sock == -1 && errno != EAGAIN && errno != EWOULDBLOCK) {
-    ELOG("Failed to accept TCP client: %s", strerror(errno));
+
+  // Set non-blocking mode for macOS compatibility (Linux accept4 does this atomically)
+  const int flags = fcntl(client_sock, F_GETFL, 0);
+  if (flags == -1) {
+    ELOG("Error getting TCP client socket flags: %s (%d), dropping client",
+         strerror(errno), errno);
+    close(client_sock);
+    return;
+  }
+  if (fcntl(client_sock, F_SETFL, flags | O_NONBLOCK) == -1) {
+    ELOG("Error setting TCP client socket to non-blocking: %s (%d), dropping client",
+         strerror(errno), errno);
+    close(client_sock);
     return;
   }
 
@@ -329,6 +335,7 @@ static void tcp_respond(dns_listener_t *self, struct sockaddr *raddr,
     WLOG("Malformed response received, invalid length: %u", resp_len);
     return;
   }
+  const uint16_t response_id = ntohs(*((uint16_t*)resp));
 
   // find client data
   struct tcp_client_s *client = NULL;
@@ -339,7 +346,6 @@ static void tcp_respond(dns_listener_t *self, struct sockaddr *raddr,
     }
   }
   if (client == NULL) {
-    uint16_t response_id = ntohs(*((uint16_t*)resp));
     WLOG("Could not find client, can not send DNS response: %04hX", response_id);
     return;
   }
@@ -355,37 +361,38 @@ static void tcp_respond(dns_listener_t *self, struct sockaddr *raddr,
   uint16_t resp_size = htons((uint16_t)resp_len);
   ssize_t len = send(client->sock, &resp_size, sizeof(uint16_t), MSG_MORE | MSG_NOSIGNAL);
   if (len != sizeof(uint16_t)) {
-    WLOG_CLIENT("Send error: %s, len: %d", strerror(errno), len);
+    WLOG_CLIENT("Send error: %s (%d), len: %d, dropping client", strerror(errno), errno, len);
     remove_client(client);
     return;
   }
 
   // send the response
   ssize_t sent = 0;
   int attempts = 0;
-  for (; attempts < 50; ++attempts)  // 25ms max wait
+  for (; attempts < RESPONSE_SEND_ATTEMPTS; ++attempts)
   {
     len = send(client->sock, resp + sent, resp_len - (size_t)sent, MSG_NOSIGNAL);
-    if (len < 0) {
+    if (len > 0) {
+      sent += len;
+      if (sent == (ssize_t)resp_len) {
+        break;
+      }
+    } else if (len < 0) {
       if (errno != EAGAIN && errno != EWOULDBLOCK) {
-        WLOG_CLIENT("Send error: %s", strerror(errno));
+        WLOG_CLIENT("Send error: %s (%d), dropping client", strerror(errno), errno);
         remove_client(client);
         return;
       }
-      // EAGAIN/EWOULDBLOCK - socket buffer full, retry after delay
-      continue;
-    }
-    sent += len;
-    if (sent == (ssize_t)resp_len) {
-      break;
     }
-    usleep(RESEND_DELAY_US);
+    usleep(RESPONSE_SEND_DELAY_US);
   }
   if (sent != (ssize_t)resp_len) {
-    WLOG_CLIENT("Send timeout after %d attempts, sent %zd/%zu bytes", attempts, sent, resp_len);
+    WLOG_CLIENT("Send timeout after %d attempts, sent %zd/%zu bytes, dropping client",
+                attempts, sent, resp_len);
     remove_client(client);
     return;
   }
+  DLOG_CLIENT("Responded %04hX", response_id);
 
   ev_timer_again(d->loop, &client->timer_watcher);
 }

src/dns_listener_udp.c

@@ -44,7 +44,6 @@ static int get_listen_sock(struct addrinfo *listen_addrinfo) {
 
   int res = bind(sock, listen_addrinfo->ai_addr, listen_addrinfo->ai_addrlen);
   if (res < 0) {
-    close(sock);
     FLOG("Error binding on %s:%d UDP: %s (%d)", ipstr, port,
          strerror(errno), errno);
   }

src/dns_poller.c

@@ -57,7 +57,7 @@ static char *get_addr_listing(struct ares_addrinfo_node * nodes) {
     const char *res = NULL;
 
     // Check that we have space for at least one character plus null terminator
-    if (pos >= list + POLLER_ADDR_LIST_SIZE - 1) {
+    if ((pos - list) >= POLLER_ADDR_LIST_SIZE - 1) {
       DLOG("Not enough space for more addresses");
       break;
     }
@@ -81,7 +81,7 @@ static char *get_addr_listing(struct ares_addrinfo_node * nodes) {
     if (res != NULL) {
       pos += strlen(pos);
       // Check we have room for the comma and null terminator
-      if (pos >= list + POLLER_ADDR_LIST_SIZE - 1) {
+      if ((pos - list) >= POLLER_ADDR_LIST_SIZE - 1) {
         DLOG("Not enough space for comma separator");
         break;
       }

src/dns_truncate.c

@@ -110,17 +110,19 @@ static void truncate_to_size_limit(char *buf, size_t *buflen, const uint16_t siz
   }
   ares_dns_record_destroy(dnsrec);
 
-  if (new_resp != NULL) {
-    if (new_resp_len < old_size) {
-      memcpy(buf, new_resp, new_resp_len);
-      *buflen = new_resp_len;
-      buf[2] |= 0x02;  // set truncation flag
-      ILOG("%04hX: DNS response size truncated from %u to %u to keep %u limit",
-           tx_id, old_size, new_resp_len, size_limit);
-    }
-    ares_free_string(new_resp);
-    new_resp = NULL;
+  if (new_resp == NULL) {
+    return;
   }
+
+  if (new_resp_len < old_size) {
+    memcpy(buf, new_resp, new_resp_len);
+    *buflen = new_resp_len;
+    buf[2] |= 0x02;  // set truncation flag
+    ILOG("%04hX: DNS response size truncated from %u to %u to keep %u limit",
+         tx_id, old_size, new_resp_len, size_limit);
+  }
+
+  ares_free_string(new_resp);
 }
 
 void dns_truncate_for_udp(const char *dns_req, size_t dns_req_len,

src/https_client.c

@@ -265,9 +265,11 @@ static void https_set_request_version(https_client_t *client,
              http_version_str(http_version_int), easy_code, curl_easy_strerror(easy_code));
 
     if (client->opt->use_http_version == 3) {
-      ELOG("Try to run application without -q argument!");
+      ELOG("Try to run application without -q argument! Falling back to HTTP/2 version.");
+      client->opt->use_http_version = 2;
     } else if (client->opt->use_http_version == 2) {
       ELOG("Try to run application with -x argument! Falling back to HTTP/1.1 version.");
+      client->opt->use_http_version = 1;
     }
   }
 }

src/logging.c

@@ -19,7 +19,6 @@ static int loglevel = LOG_ERROR;                     // NOLINT(cppcoreguidelines
 static ev_timer logging_timer;                       // NOLINT(cppcoreguidelines-avoid-non-const-global-variables)
 static ev_signal sigusr2;                            // NOLINT(cppcoreguidelines-avoid-non-const-global-variables)
 static ev_async flight_recorder_async;               // NOLINT(cppcoreguidelines-avoid-non-const-global-variables)
-static struct ev_loop *logging_loop = NULL;          // NOLINT(cppcoreguidelines-avoid-non-const-global-variables)
 static struct ring_buffer * flight_recorder = NULL;  // NOLINT(cppcoreguidelines-avoid-non-const-global-variables)
 
 static const char * const SeverityStr[] = {
@@ -54,17 +53,15 @@ static void logging_flight_recorder_dump_async_cb(struct ev_loop __attribute__((
   logging_flight_recorder_dump();
 }
 
-static void logging_flight_recorder_dump_cb(struct ev_loop __attribute__((unused)) *loop,
+static void logging_flight_recorder_dump_cb(struct ev_loop *loop,
     ev_signal __attribute__((__unused__)) *w,
     int __attribute__((__unused__)) revents) {
   // Signal handler: just trigger async watcher to defer to main loop
   // This ensures fprintf() is called outside of signal context
-  ev_async_send(logging_loop, &flight_recorder_async);
+  ev_async_send(loop, &flight_recorder_async);
 }
 
 void logging_events_init(struct ev_loop *loop) {
-  logging_loop = loop;
-
   /* don't start timer if we will never write messages that are not flushed */
   if (loglevel < LOG_FLUSH_LEVEL) {
     DLOG("starting periodic log flush timer");

src/main.c

@@ -141,8 +141,9 @@ int main(int argc, char *argv[]) {
       exit(0);  // asking for help is not a problem
     case OPR_VERSION: {
       printf("%s\n", sw_version());
+      CURLcode init_res = curl_global_init(CURL_GLOBAL_DEFAULT);  // needed to ensure, that curl_version*() calls will work properly!
       curl_version_info_data *curl_ver = curl_version_info(CURLVERSION_NOW);
-      if (curl_ver != NULL) {
+      if (init_res == CURLE_OK && curl_ver != NULL) {
         printf("Using: ev/%d.%d c-ares/%s %s\n",
                ev_version_major(), ev_version_minor(),
                ares_version(NULL), curl_version());
@@ -151,6 +152,7 @@ int main(int argc, char *argv[]) {
                curl_ver->features & CURL_VERSION_HTTP3 ? "HTTP3 " : "",
                curl_ver->features & CURL_VERSION_HTTPS_PROXY ? "HTTPS-proxy " : "",
                curl_ver->features & CURL_VERSION_IPV6 ? "IPv6" : "");
+        curl_global_cleanup();
         exit(0);
       } else {
         printf("\nFailed to get curl version info!\n");

src/ring_buffer.c

@@ -5,7 +5,9 @@
 
 #include "ring_buffer.h"
 
-#define MAX_LOG_ENTRY_SIZE 8192
+enum {
+MAX_LOG_ENTRY_SIZE = 8192
+};
 
 struct ring_buffer
 {
@@ -23,13 +25,11 @@ void ring_buffer_init(struct ring_buffer **rbp, uint32_t size)
     }
     struct ring_buffer *rb = (struct ring_buffer *)calloc(1, sizeof(struct ring_buffer));
     if (!rb) {
-        *rbp = NULL;
         return;
     }
     rb->storage = (char**)calloc(size, sizeof(char*));
     if (!rb->storage) {
         free((void*) rb);
-        *rbp = NULL;
         return;
     }
     rb->size = size;