Re: [PATCH v2 07/14] global: trivial conversions to fix `-Wsign-compare` warnings
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Dec 4, 2024, 05:31 UTC
- Message-ID
- <xmqqcyi83tfp.fsf@gitster.g>
- In-Reply-To
- <20241202-pks-sign-compare-v2-7-e7f0ad92a749@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
Show 27 quoted lines
> 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. */