Skip to content

Commit 25435e4

Browse files
pks-tgitster
authored andcommitted
pkt-line: fix -Wsign-compare warning on 32 bit platform
Similar to the preceding commit, we get a warning in `get_packet_data()` on 32 bit platforms due to our lenient use of `ssize_t`. This function is kind of curious though: we accept an `unsigned size` of bytes to read, then store the actual number of bytes read in an `ssize_t` and return it as an `int`. This is a whole lot of integer conversions, and in theory these can cause us to overflow when the passed-in size is larger than `ssize_t`, which on 32 bit platforms is implemented as an `int`. None of the callers of that function even care about the number of bytes we have read, so returning that number is moot anyway. Refactor the function such that it only returns an error code, which plugs the potential overflow. While at it, convert the passed-in size parameter to be of type `size_t`. Signed-off-by: Patrick Steinhardt <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent ba8f601 commit 25435e4

File tree

1 file changed

+11
-9
lines changed

1 file changed

+11
-9
lines changed

pkt-line.c

+11-9
Original file line numberDiff line numberDiff line change
@@ -340,30 +340,32 @@ int write_packetized_from_buf_no_flush_count(const char *src_in, size_t len,
340340
}
341341

342342
static int get_packet_data(int fd, char **src_buf, size_t *src_size,
343-
void *dst, unsigned size, int options)
343+
void *dst, size_t size, int options)
344344
{
345-
ssize_t ret;
345+
size_t bytes_read;
346346

347347
if (fd >= 0 && src_buf && *src_buf)
348348
BUG("multiple sources given to packet_read");
349349

350350
/* Read up to "size" bytes from our source, whatever it is. */
351351
if (src_buf && *src_buf) {
352-
ret = size < *src_size ? size : *src_size;
353-
memcpy(dst, *src_buf, ret);
354-
*src_buf += ret;
355-
*src_size -= ret;
352+
bytes_read = size < *src_size ? size : *src_size;
353+
memcpy(dst, *src_buf, bytes_read);
354+
*src_buf += bytes_read;
355+
*src_size -= bytes_read;
356356
} else {
357-
ret = read_in_full(fd, dst, size);
357+
ssize_t ret = read_in_full(fd, dst, size);
358358
if (ret < 0) {
359359
if (options & PACKET_READ_GENTLE_ON_READ_ERROR)
360360
return error_errno(_("read error"));
361361
die_errno(_("read error"));
362362
}
363+
364+
bytes_read = (size_t) ret;
363365
}
364366

365367
/* And complain if we didn't get enough bytes to satisfy the read. */
366-
if (ret != size) {
368+
if (bytes_read != size) {
367369
if (options & PACKET_READ_GENTLE_ON_EOF)
368370
return -1;
369371

@@ -372,7 +374,7 @@ static int get_packet_data(int fd, char **src_buf, size_t *src_size,
372374
die(_("the remote end hung up unexpectedly"));
373375
}
374376

375-
return ret;
377+
return 0;
376378
}
377379

378380
int packet_length(const char lenbuf_hex[4], size_t size)

0 commit comments

Comments
 (0)