[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [Qemu-devel] [PATCH v4 04/24] memory-device: handle integ
From: |
Igor Mammedov |
Subject: |
Re: [Qemu-ppc] [Qemu-devel] [PATCH v4 04/24] memory-device: handle integer overflows properly |
Date: |
Thu, 27 Sep 2018 10:02:04 +0200 |
On Wed, 26 Sep 2018 11:41:59 +0200
David Hildenbrand <address@hidden> wrote:
> Make address_space_end point at the real end, instead of end + 1, so we don't
> have to handle special cases like it being 0. This will allow us to
> place a memory device at the very end of the guest physical 64bit address
> space (if ever possible).
[...]
> @@ -115,7 +116,7 @@ uint64_t memory_device_get_free_addr(MachineState *ms,
> const uint64_t *hint,
> }
> address_space_start = ms->device_memory->base;
> address_space_end = address_space_start +
> - memory_region_size(&ms->device_memory->mr);
> + memory_region_size(&ms->device_memory->mr) - 1;
I'm terrible at math, lets assume size = 1 so after this
'real end' would end up at 'address_space_start', which makes
it rather confusing beside not matching variable name anymore.
I'd drop it and maybe extend the following assert to abort
on overflow condition.
> g_assert(address_space_end >= address_space_start);
>
> /* address_space_start indicates the maximum alignment we expect */
> @@ -149,7 +150,8 @@ uint64_t memory_device_get_free_addr(MachineState *ms,
> const uint64_t *hint,
> "] before 0x%" PRIx64, new_addr, size,
> address_space_start);
> return 0;
> - } else if ((new_addr + size) > address_space_end) {
> + } else if (new_addr + size - 1 < new_addr ||
> + new_addr + size - 1 > address_space_end) {
> error_setg(errp, "can't add memory [0x%" PRIx64 ":0x%" PRIx64
> "] beyond 0x%" PRIx64, new_addr, size,
> address_space_end);
> @@ -182,7 +184,8 @@ uint64_t memory_device_get_free_addr(MachineState *ms,
> const uint64_t *hint,
> }
> }
>
> - if (new_addr + size > address_space_end) {
> + if (new_addr + size - 1 < new_addr || !new_addr ||
> + new_addr + size - 1 > address_space_end) {
> error_setg(errp, "could not find position in guest address space for
> "
> "memory device - memory fragmented due to alignments");
> goto out;
I strongly suggest replace non obvious math with several
plain <>= conditions even if adds more conditions compared
to math variant.
At least reader doesn't have to do calculations manually
to figure out what's going on
- [Qemu-ppc] [PATCH v4 01/24] memory-device: fix alignment error message, (continued)
- [Qemu-ppc] [PATCH v4 01/24] memory-device: fix alignment error message, David Hildenbrand, 2018/09/26
- [Qemu-ppc] [PATCH v4 02/24] memory-device: fix error message when hinted address is too small, David Hildenbrand, 2018/09/26
- [Qemu-ppc] [PATCH v4 03/24] pc-dimm: pass PCDIMMDevice to pc_dimm_.*plug, David Hildenbrand, 2018/09/26
- [Qemu-ppc] [PATCH v4 04/24] memory-device: handle integer overflows properly, David Hildenbrand, 2018/09/26
- Re: [Qemu-ppc] [Qemu-devel] [PATCH v4 04/24] memory-device: handle integer overflows properly,
Igor Mammedov <=
- Re: [Qemu-ppc] [Qemu-devel] [PATCH v4 04/24] memory-device: handle integer overflows properly, David Hildenbrand, 2018/09/27
- Re: [Qemu-ppc] [Qemu-devel] [PATCH v4 04/24] memory-device: handle integer overflows properly, Igor Mammedov, 2018/09/27
- Re: [Qemu-ppc] [Qemu-devel] [PATCH v4 04/24] memory-device: handle integer overflows properly, David Hildenbrand, 2018/09/27
- Re: [Qemu-ppc] [Qemu-devel] [PATCH v4 04/24] memory-device: handle integer overflows properly, Igor Mammedov, 2018/09/27
- Re: [Qemu-ppc] [Qemu-devel] [PATCH v4 04/24] memory-device: handle integer overflows properly, David Hildenbrand, 2018/09/27
- Re: [Qemu-ppc] [Qemu-devel] [PATCH v4 04/24] memory-device: handle integer overflows properly, Igor Mammedov, 2018/09/27
[Qemu-ppc] [PATCH v4 05/24] memory-device: use memory device terminology in error messages, David Hildenbrand, 2018/09/26
[Qemu-ppc] [PATCH v4 06/24] memory-device: introduce separate config option, David Hildenbrand, 2018/09/26
[Qemu-ppc] [PATCH v4 07/24] memory-device: forward errors in get_region_size()/get_plugged_size(), David Hildenbrand, 2018/09/26