On Mon, Aug 19, 2019 at 05:32:14AM +0000, Zeng, Star wrote: > > >> -----Original Message----- >> From: Wei Yang [mailto:richardw.y...@linux.intel.com] >> Sent: Monday, August 19, 2019 10:39 AM >> To: David Hildenbrand <da...@redhat.com> >> Cc: Wei Yang <richardw.y...@linux.intel.com>; Zeng, Star >> <star.z...@intel.com>; imamm...@redhat.com; qemu-devel@nongnu.org; >> 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 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. > >It was just my suggestion. I am fine with any preference you and other experts >have. >
Thanks >Thanks, >Star > >> >> >-- >> > >> >Thanks, >> > >> >David / dhildenb >> >> -- >> Wei Yang >> Help you, Help me -- Wei Yang Help you, Help me
