On 08.06.2018 09:48, David Hildenbrand wrote: > On 08.06.2018 09:46, Greg Kurz wrote: >> On Fri, 8 Jun 2018 09:42:48 +0200 >> David Hildenbrand <da...@redhat.com> wrote: >> >>> On 08.06.2018 09:34, Greg Kurz wrote: >>>> On Thu, 7 Jun 2018 18:52:12 +0200 >>>> David Hildenbrand <da...@redhat.com> wrote: >>>> >>>>> The node property can always be queried and the value has already been >>>>> verified in pc_dimm_realize(). >>>>> >>>>> Signed-off-by: David Hildenbrand <da...@redhat.com> >>>>> --- >>>>> hw/ppc/spapr.c | 9 +-------- >>>>> 1 file changed, 1 insertion(+), 8 deletions(-) >>>>> >>>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >>>>> index 2375cbee12..d038f3243e 100644 >>>>> --- a/hw/ppc/spapr.c >>>>> +++ b/hw/ppc/spapr.c >>>>> @@ -3578,14 +3578,7 @@ static void >>>>> spapr_machine_device_plug(HotplugHandler *hotplug_dev, >>>>> error_setg(errp, "Memory hotplug not supported for this >>>>> machine"); >>>>> return; >>>>> } >>>>> - node = object_property_get_uint(OBJECT(dev), PC_DIMM_NODE_PROP, >>>>> errp); >>>>> - if (*errp) { >>>> >>>> Good riddance :) >>>> >>>>> - return; >>>>> - } >>>>> - if (node < 0 || node >= MAX_NODES) { >>>>> - error_setg(errp, "Invaild node %d", node); >>>>> - return; >>>>> - }
Maybe turn that into an assert() instead? ... just for the paranoids ;-) >>>>> + node = object_property_get_uint(OBJECT(dev), PC_DIMM_NODE_PROP, >>>>> NULL); >>>> >>>> Maybe pass &error_abort ? >>> >>> I'm using the same access scheme as in hw/acpi/memory_hotplug.c >>> >>> ("error ignored" vs. "error leads to an abort") - but this will actually >>> never fail. But I can use error_abort here, does not matter. >>> >> >> Heh, /me paranoid but this is David's call and he acked that already >> so I guess it's okay. > > NULL makes it fit into a single line :) +1 for error_abort, even if it takes another line. Thomas