Am 27.10.2015 um 00:45 hat John Snow geschrieben:
> cvtnum() returns int64_t: we should not be storing this
> result inside of an int.
>
> In a few cases, we need an extra sprinkling of error handling
> where we expect to pass this number on towards a function that
> expects something smaller than int64_t.
>
> Reported-by: Max Reitz <mre...@redhat.com>
> Signed-off-by: John Snow <js...@redhat.com>
> Reviewed-by: Eric Blake <ebl...@redhat.com>
> ---
> qemu-io-cmds.c | 31 +++++++++++++++++--------------
> 1 file changed, 17 insertions(+), 14 deletions(-)
>
> diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
> index 6e5d1e4..704db89 100644
> --- a/qemu-io-cmds.c
> +++ b/qemu-io-cmds.c
> @@ -642,10 +642,11 @@ static int read_f(BlockBackend *blk, int argc, char
> **argv)
> int c, cnt;
> char *buf;
> int64_t offset;
> - int count;
> + int64_t count;
> /* Some compilers get confused and warn if this is not initialized. */
> int total = 0;
> - int pattern = 0, pattern_offset = 0, pattern_count = 0;
> + int pattern = 0;
> + int64_t pattern_offset = 0, pattern_count = 0;
>
> while ((c = getopt(argc, argv, "bCl:pP:qs:v")) != -1) {
> switch (c) {
> @@ -734,7 +735,7 @@ static int read_f(BlockBackend *blk, int argc, char
> **argv)
> return 0;
> }
> if (count & 0x1ff) {
> - printf("count %d is not sector aligned\n",
> + printf("count %"PRId64" is not sector aligned\n",
> count);
> return 0;
> }
> @@ -762,7 +763,7 @@ static int read_f(BlockBackend *blk, int argc, char
> **argv)
> memset(cmp_buf, pattern, pattern_count);
> if (memcmp(buf + pattern_offset, cmp_buf, pattern_count)) {
> printf("Pattern verification failed at offset %"
> - PRId64 ", %d bytes\n",
> + PRId64 ", %"PRId64" bytes\n",
> offset + pattern_offset, pattern_count);
> }
> g_free(cmp_buf);
read_f calls a few helper function which only take an int for count:
do_pread(), do_load_vmstate(), do_read() actually perform the request.
These should probably take int64_t as well (and if we want to be really
careful to avoid wraparounds, check limits individually).
qemu_io_alloc() takes size_t, so will wrap around on 32 bit hosts.
Should take int64_t and check against SIZE_MAX.
dump_buffer() also only takes an int, but I hope nobody tries to dump
more than 2 GB...
print_report() should probably be fixed to take int64_t.
And for total to make sense, it probably needs to be converted to
int64_t as well.
> @@ -957,7 +958,7 @@ static int write_f(BlockBackend *blk, int argc, char
> **argv)
> int c, cnt;
> char *buf = NULL;
> int64_t offset;
> - int count;
> + int64_t count;
> /* Some compilers get confused and warn if this is not initialized. */
> int total = 0;
> int pattern = 0xcd;
> @@ -1029,7 +1030,7 @@ static int write_f(BlockBackend *blk, int argc, char
> **argv)
> }
>
> if (count & 0x1ff) {
> - printf("count %d is not sector aligned\n",
> + printf("count %"PRId64" is not sector aligned\n",
> count);
> return 0;
> }
For writes, the helper functions to perform the request are different,
but they also only take int: do_pwrite(), do_save_vmstate(),
do_co_write_zeroes(), do_write_compressed(), do_write().
The rest should be fixed when you fix the helpers for read.
> @@ -1777,8 +1778,7 @@ static int discard_f(BlockBackend *blk, int argc, char
> **argv)
> struct timeval t1, t2;
> int Cflag = 0, qflag = 0;
> int c, ret;
> - int64_t offset;
> - int count;
> + int64_t offset, count;
>
> while ((c = getopt(argc, argv, "Cq")) != -1) {
> switch (c) {
Here, blk_discard() is called directly without a helper function. A
check that the number of sectors fits in an int is missing.
> @@ -1833,11 +1833,10 @@ out:
> static int alloc_f(BlockBackend *blk, int argc, char **argv)
> {
> BlockDriverState *bs = blk_bs(blk);
> - int64_t offset, sector_num;
> - int nb_sectors, remaining;
> + int64_t offset, sector_num, nb_sectors, remaining;
> char s1[64];
> - int num, sum_alloc;
> - int ret;
> + int num, ret;
> + int64_t sum_alloc;
>
> offset = cvtnum(argv[1]);
> if (offset < 0) {
> @@ -1881,7 +1880,7 @@ static int alloc_f(BlockBackend *blk, int argc, char
> **argv)
>
> cvtstr(offset, s1, sizeof(s1));
>
> - printf("%d/%d sectors allocated at offset %s\n",
> + printf("%"PRId64"/%"PRId64" sectors allocated at offset %s\n",
> sum_alloc, nb_sectors, s1);
> return 0;
> }
remaining is passed to bdrv_is_allocated() without checking against
INT_MAX first.
Kevin
