Re: [Qemu-devel] [PATCH] pc-dimm: use same mechanism for [get|set]_addr

[Wei Yang]

On Wed, Feb 06, 2019 at 12:59:49PM +0100, Igor Mammedov wrote:
>On Fri,  1 Feb 2019 09:08:37 +0800
>Wei Yang <address@hidden> wrote:
>
>> [get|set]_addr are two counterpart to access PCDIMMDevice.addr.
>> 
>> Since we have already set up a property PC_DIMM_ADDR_PROP for this
>> field and use this mechanism in set_addr, it would be more proper to use
>> the same mechanism in set_addr.
>> 
>> This patch uses object_property_get_uint() to replace the direct memory
>> access to make [get|set]_addr with the same mechanism.
>> 
>> Signed-off-by: Wei Yang <address@hidden>
>> ---
>>  hw/mem/pc-dimm.c | 4 +---
>>  1 file changed, 1 insertion(+), 3 deletions(-)
>> 
>> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
>> index 0c9b9e8292..c0658b9b88 100644
>> --- a/hw/mem/pc-dimm.c
>> +++ b/hw/mem/pc-dimm.c
>> @@ -204,9 +204,7 @@ static MemoryRegion 
>> *pc_dimm_get_memory_region(PCDIMMDevice *dimm, Error **errp)
>>  
>>  static uint64_t pc_dimm_md_get_addr(const MemoryDeviceState *md)
>>  {
>> -    const PCDIMMDevice *dimm = PC_DIMM(md);
>> -
>> -    return dimm->addr;
>> +    return object_property_get_uint(OBJECT(md), PC_DIMM_ADDR_PROP, NULL);
>it's not good to ignore errors
>
>s/NULL/error_abort/
>
>so we would notice if error ever happened.

You mean something like this?

        Error *err;
        uint64_t addr;

        addr = object_property_get_uint(OBJECT(md), PC_DIMM_ADDR_PROP, &err);

        if (err) {
                warn_report_err(&err);
                return 0;
        }

        return addr;

>
>>  }
>>  
>>  static void pc_dimm_md_set_addr(MemoryDeviceState *md, uint64_t addr,
>

-- 
Wei Yang
Help you, Help me