Re: [Qemu-devel] [PATCH 01/23] memory: introduce memory_region_find()

[Avi Kivity <avi@redhat.com>]

On 12/19/2011 04:50 PM, Anthony Liguori wrote:
>> +static int cmp_flatrange_addr(const void *_addr, const void *_fr)
>> +{
>> +    const AddrRange *addr = _addr;
>> +    const FlatRange *fr = _fr;
>
>
> Please don't prefix with an underscore.

Why not?  It's legal according to the standards, if that's your concern
(only _[_A-Z]+ are reserved).

>> @@ -502,6 +520,20 @@ void memory_region_del_subregion(MemoryRegion *mr,
>>                                    MemoryRegion *subregion);
>>
>>   /**
>> + * memory_region_find: locate a MemoryRegion in an address space
>> + *
>> + * Locates the first #MemoryRegion within an address space given by
>> + * @address_space that overlaps the range given by @addr and @size.
>> + *
>> + * @address_space: a top-level (i.e. parentless) region that contains
>> + *       the region to be found
>> + * @addr: start of the area within @address_space to be searched
>> + * @size: size of the area to be searched
>> + */
>> +MemoryRegionSection memory_region_find(MemoryRegion *address_space,
>> +                                       target_phys_addr_t addr,
>> uint64_t size);
>
>
> Returning structs by value is a bit unexpected.

It's just prejudice, here's the call sequence:

  127a63:       48 89 c6                mov    %rax,%rsi
  127a66:       48 8d 45 b0             lea    -0x50(%rbp),%rax
  127a6a:       b9 01 00 00 00          mov    $0x1,%ecx
  127a6f:       48 89 da                mov    %rbx,%rdx
  127a72:       48 89 c7                mov    %rax,%rdi
  127a75:       e8 89 46 15 00          callq  27c103 <memory_region_find>

The return value is passed via %rax, so no performance penalty.  On the
other hand, it's clear that it is a return value, unlike a pointer
parameter.

-- 
error compiling committee.c: too many arguments to function