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. -- Thanks, David / dhildenb
