Re: [PATCH v2 07/14] global: trivial conversions to fix `-Wsign-compare` warnings

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

> diff --git a/pkt-line.c b/pkt-line.c
> index 90ea2b6974b1d0957cfdc5e2f9a2c30720723f12..1c7e8d826d10b3125962e14476c24771d82888b1 100644
> --- a/pkt-line.c
> +++ b/pkt-line.c
> @@ -1,5 +1,3 @@
> -#define DISABLE_SIGN_COMPARE_WARNINGS
> -
>  #include "git-compat-util.h"
>  #include "copy.h"
>  #include "pkt-line.h"
> @@ -41,7 +39,6 @@ static int packet_trace_pack(const char *buf, unsigned int len, int sideband)
>  
>  static void packet_trace(const char *buf, unsigned int len, int write)
>  {
> -	int i;
>  	struct strbuf out;
>  	static int in_pack, sideband;
>  
> @@ -74,7 +71,7 @@ static void packet_trace(const char *buf, unsigned int len, int write)
>  		    get_trace_prefix(), write ? '>' : '<');
>  
>  	/* XXX we should really handle printable utf8 */
> -	for (i = 0; i < len; i++) {
> +	for (unsigned int i = 0; i < len; i++) {
>  		/* suppress newlines */
>  		if (buf[i] == '\n')
>  			continue;

This unfortunately is not enough for 32-bit architectures.

As the helper is limited to handle only 64kB worth of data, "size"
being "unsigned" is more than sufficiently wide even on 32-bit box,
and a variable of type ssize_t is still wide enough, but the
compilers would not know that.

I do not have a 32-bit box handy, so this is not tested in the
target environment the fix aims at, but at least I tested to
make sure it does not break 64-bit builds.

It seems that csum-file.c has a similar issue, but I haven't had a
chance to take a look.

 pkt-line.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git c/pkt-line.c w/pkt-line.c
index 1c7e8d826d..d08d46a99b 100644
--- c/pkt-line.c
+++ w/pkt-line.c
@@ -339,7 +339,7 @@ int write_packetized_from_buf_no_flush_count(const char *src_in, size_t len,
 static int get_packet_data(int fd, char **src_buf, size_t *src_size,
 			   void *dst, unsigned size, int options)
 {
-	ssize_t ret;
+	size_t ret;
 
 	if (fd >= 0 && src_buf && *src_buf)
 		BUG("multiple sources given to packet_read");
@@ -351,12 +351,13 @@ static int get_packet_data(int fd, char **src_buf, size_t *src_size,
 		*src_buf += ret;
 		*src_size -= ret;
 	} else {
-		ret = read_in_full(fd, dst, size);
-		if (ret < 0) {
+		ssize_t num_read = read_in_full(fd, dst, size);
+		if (num_read < 0) {
 			if (options & PACKET_READ_GENTLE_ON_READ_ERROR)
 				return error_errno(_("read error"));
 			die_errno(_("read error"));
 		}
+		ret = num_read;
 	}
 
 	/* And complain if we didn't get enough bytes to satisfy the read. */