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. >Thanks, >Star > >> >> > >> >Thanks, >> >Star >> > >> >> } >> >> out: >> >> g_slist_free(list); >> >> -- >> >> 2.17.1 >> >> >> >> -- >> Wei Yang >> Help you, Help me -- Wei Yang Help you, Help me
