dnsproxy: fix signedness warnings

Commit Message

Brian Fukano Feb. 9, 2024, 6:01 p.m. UTC

This fixes the signdness warnings in dnsproxy.c
---
 src/dnsproxy.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Comments

Marcel Holtmann Feb. 13, 2024, 8:42 a.m. UTC | #1

Hi Brian,

> This fixes the signdness warnings in dnsproxy.c
> ---
> src/dnsproxy.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/src/dnsproxy.c b/src/dnsproxy.c
> index d4242560..72e77f96 100644
> --- a/src/dnsproxy.c
> +++ b/src/dnsproxy.c
> @@ -436,7 +436,7 @@ static void update_cached_ttl(unsigned char *ptr, int len, int new_ttl)
> ptr += DNS_HEADER_SIZE;
> len -= DNS_HEADER_SIZE;
> 
> - if (len < DNS_QUESTION_SIZE + 1)
> + if (len < 0 || (unsigned int)len < DNS_QUESTION_SIZE + 1)
> return;

I love that you are trying to address these warnings. However just casting the issue away is not really helpful. Lets us proper types for the variables and have the checks make sense.

Background is that I want the compiler to keep warning us about these. Once you cast, the warning is gone forever.

Regards

Marcel

Patch

diff --git a/src/dnsproxy.c b/src/dnsproxy.c
index d4242560..72e77f96 100644
--- a/src/dnsproxy.c
+++ b/src/dnsproxy.c
@@ -436,7 +436,7 @@  static void update_cached_ttl(unsigned char *ptr, int len, int new_ttl)
 	ptr += DNS_HEADER_SIZE;
 	len -= DNS_HEADER_SIZE;
 
-	if (len < DNS_QUESTION_SIZE + 1)
+	if (len < 0 || (unsigned int)len < DNS_QUESTION_SIZE + 1)
 		return;
 
 	/* skip the query, which is a name and a struct domain_question */
@@ -459,7 +459,7 @@  static void update_cached_ttl(unsigned char *ptr, int len, int new_ttl)
 			break;
 
 		rr = (void*)ptr;
-		if (len < sizeof(*rr))
+		if ((unsigned int)len < sizeof(*rr))
 			/* incomplete record */
 			break;
 
@@ -520,7 +520,7 @@  static void send_cached_response(int sk, const unsigned char *ptr, size_t len,
 		connman_error("Cannot send cached DNS response: %s",
 				strerror(errno));
 	}
-	else if (err != len || dns_len != (len - offset))
+	else if ((unsigned int)err != len || dns_len != (len - offset))
 		debug("Packet length mismatch, sent %d wanted %zd dns %zd",
 			err, len, dns_len);
 }
@@ -656,7 +656,7 @@  static int append_data(unsigned char *buf, size_t size, const char *data)
 
 	while (true) {
 		const char *dot = strchr(data, '.');
-		len = dot ? dot - data : strlen(data);
+		len = dot ? (unsigned int)(dot - data) : strlen(data);
 
 		if (len == 0)
 			break;
@@ -1063,7 +1063,7 @@  static int parse_response(const unsigned char *buf, size_t buflen,
 	qlen = strlen(question);
 	ptr += qlen + 1; /* skip \0 */
 
-	if ((eptr - ptr) < DNS_QUESTION_SIZE)
+	if ((unsigned int)(eptr - ptr) < DNS_QUESTION_SIZE)
 		return -EINVAL;
 
 	q = (void *) ptr;
@@ -2031,7 +2031,7 @@  static int dns_reply_fixup_domains(
 	const char *domain;
 
 	/* full header plus at least one byte for the hostname length */
-	if (reply_len < header_len + 1)
+	if (reply_len < (unsigned int)(header_len + 1))
 		return -EINVAL;
 
 	section_counts[0] = hdr->ancount;
@@ -2521,7 +2521,7 @@  hangup:
 				connman_error("DNS proxy error %s",
 						strerror(errno));
 				goto hangup;
-			} else if (bytes_recv < sizeof(reply_len))
+			} else if ((unsigned int)bytes_recv < sizeof(reply_len))
 				return TRUE;
 
 			/* the header contains the length of the message