Glauber Costa wrote:
> On Wed, Sep 10, 2008 at 09:23:18PM +0200, Jan Kiszka wrote:
>> Glauber Costa wrote:
>>> From: Glauber Costa <[EMAIL PROTECTED]>
>>>
>>> kvm_cpu_register_physical_memory() is its only user. Remove it.
>>>
>>> Signed-off-by: Glauber Costa <[EMAIL PROTECTED]>
>>> ---
>>> qemu/qemu-kvm.c | 52 +++++++++++++++++++++-------------------------------
>>> 1 files changed, 21 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/qemu/qemu-kvm.c b/qemu/qemu-kvm.c
>>> index 8d366e5..f0ef21e 100644
>>> --- a/qemu/qemu-kvm.c
>>> +++ b/qemu/qemu-kvm.c
>>> @@ -775,42 +775,32 @@ void
>>> kvm_cpu_register_physical_memory(target_phys_addr_t start_addr,
>>> unsigned long size,
>>> unsigned long phys_offset)
>>> {
>>> -#ifdef KVM_CAP_USER_MEMORY
>>> int r = 0;
>>> -
>>> - r = kvm_check_extension(kvm_context, KVM_CAP_USER_MEMORY);
>>> - if (r) {
>>> - if (!(phys_offset & ~TARGET_PAGE_MASK)) {
>>> - r = kvm_is_allocated_mem(kvm_context, start_addr, size);
>>> - if (r)
>>> - return;
>>> - r = kvm_is_intersecting_mem(kvm_context, start_addr);
>>> - if (r)
>>> - kvm_create_mem_hole(kvm_context, start_addr, size);
>>> - r = kvm_register_userspace_phys_mem(kvm_context, start_addr,
>>> - phys_ram_base +
>>> phys_offset,
>>> - size, 0);
>>> - }
>>> - if (phys_offset & IO_MEM_ROM) {
>>> - phys_offset &= ~IO_MEM_ROM;
>>> - r = kvm_is_intersecting_mem(kvm_context, start_addr);
>>> - if (r)
>>> - kvm_create_mem_hole(kvm_context, start_addr, size);
>>> - r = kvm_register_userspace_phys_mem(kvm_context, start_addr,
>>> - phys_ram_base +
>>> phys_offset,
>>> - size, 0);
>>> - }
>>> - if (r < 0) {
>>> - printf("kvm_cpu_register_physical_memory: failed\n");
>>> - exit(1);
>>> - }
>>> - return;
>>> + if (!(phys_offset & ~TARGET_PAGE_MASK)) {
>>> + r = kvm_is_allocated_mem(kvm_context, start_addr, size);
>>> + if (r)
>>> + return;
>>> + r = kvm_is_intersecting_mem(kvm_context, start_addr);
>>> + if (r)
>>> + kvm_create_mem_hole(kvm_context, start_addr, size);
>>> + r = kvm_register_userspace_phys_mem(kvm_context, start_addr,
>>> + phys_ram_base + phys_offset,
>>> + size, 0);
>>> }
>>> -#endif
>>> if (phys_offset & IO_MEM_ROM) {
>> At this chance: Shouldn't this become 'else if'?
>
> fyi: just sent out a new series, this time with numbers and no tabs in qemu
> ;-)Yeah, much better. :) > > As a coding style practice, yes. But they are mutually exclusive anyway, so > it shouldn't > matter. We could definitely try to make it better, but they way to go is to > have only one case > instead of two: look at how close they are to each other! I have a later > series that do that, > so I'm just trying to set up the ground here. Well, they are close partly because the IO_MEM_ROM part is incomplete (it ignores the 'read-only' property). However, I just stumbled over it while trying to understand how this function works. Without the 'else', I had to check twice that the cases are exclusive in practice... Jan
signature.asc
Description: OpenPGP digital signature
