On Thu, Aug 08, 2019 at 09:06:21AM +0200, David Hildenbrand wrote: >On 08.08.19 04:38, Wei Yang wrote: >> On Thu, Aug 08, 2019 at 02:30:02AM +0000, Zeng, Star wrote: >>>> -----Original Message----- >>>> From: Wei Yang [mailto:richardw.y...@linux.intel.com] >>>> Sent: Thursday, August 8, 2019 10:13 AM >>>> To: Zeng, Star <star.z...@intel.com> >>>> Cc: Wei Yang <richardw.y...@linux.intel.com>; qemu-devel@nongnu.org; >>>> imamm...@redhat.com; da...@redhat.com; m...@redhat.com >>>> Subject: Re: [Qemu-devel] [PATCH v2 1/2] memory-device: not necessary to >>>> use goto for the last check >>>> >>>> On Thu, Aug 08, 2019 at 01:42:14AM +0000, Zeng, Star wrote: >>>>>> -----Original Message----- >>>>>> From: Qemu-devel [mailto:qemu-devel- >>>>>> bounces+star.zeng=intel....@nongnu.org] On Behalf Of Wei Yang >>>>>> Sent: Tuesday, July 30, 2019 8:38 AM >>>>>> To: qemu-devel@nongnu.org >>>>>> Cc: imamm...@redhat.com; da...@redhat.com; Wei Yang >>>>>> <richardw.y...@linux.intel.com>; m...@redhat.com >>>>>> Subject: [Qemu-devel] [PATCH v2 1/2] memory-device: not necessary to >>>>>> use goto for the last check >>>>>> >>>>>> We are already at the last condition check. >>>>>> >>>>>> Signed-off-by: Wei Yang <richardw.y...@linux.intel.com> >>>>>> Reviewed-by: Igor Mammedov <imamm...@redhat.com> >>>>>> Reviewed-by: David Hildenbrand <da...@redhat.com> >>>>>> --- >>>>>> hw/mem/memory-device.c | 1 - >>>>>> 1 file changed, 1 deletion(-) >>>>>> >>>>>> diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c >>>> index >>>>>> 5f2c408036..df3261b32a 100644 >>>>>> --- a/hw/mem/memory-device.c >>>>>> +++ b/hw/mem/memory-device.c >>>>>> @@ -186,7 +186,6 @@ static uint64_t >>>>>> memory_device_get_free_addr(MachineState *ms, >>>>>> if (!range_contains_range(&as, &new)) { >>>>>> error_setg(errp, "could not find position in guest address >>>>>> space for " >>>>>> "memory device - memory fragmented due to >>>>>> alignments"); >>>>>> - goto out; >>>>> >>>>> Is it better to return 0 (or set new_addr to 0) for this error path and >>>>> another >>>> remaining "goto out" path? >>>>> >>>> >>>> I may not get your point. >>>> >>>> We set errp which is handled in its caller. By doing so, the error is >>>> propagated. >>>> >>>> Do I miss something? >>> >>> Yes, you are right. Currently, the caller is checking errp, but not the >>> returned address, so there should be no issue. >>> But when you see other error paths, you will find they all return 0. To be >>> aligned (return 0 when error), so just suggest also returning 0 for these >>> two "goto out" error path. :) >>> >> >> You may have some point. >> >> Let's see whether others have the same taste, or we can refine it separately. >> > >I don't think that's necessary (callers really should check for errors >before using the return values), but I would also not object to that change. >
In case there is no strong requirement to refactor the code. I would leave it here. >-- > >Thanks, > >David / dhildenb -- Wei Yang Help you, Help me
