From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:56175)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <bsd@redhat.com>) id 1bNnmU-0006KC-5a
	for qemu-devel@nongnu.org; Thu, 14 Jul 2016 17:02:51 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <bsd@redhat.com>) id 1bNnmP-0001Gw-6y
	for qemu-devel@nongnu.org; Thu, 14 Jul 2016 17:02:49 -0400
Received: from mx1.redhat.com ([209.132.183.28]:42209)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <bsd@redhat.com>) id 1bNnmP-0001Gf-2G
	for qemu-devel@nongnu.org; Thu, 14 Jul 2016 17:02:45 -0400
From: Bandan Das <bsd@redhat.com>
References: <1468515285-173356-1-git-send-email-imammedo@redhat.com>
	<1468515285-173356-5-git-send-email-imammedo@redhat.com>
	<20160714175453.GZ3727@thinpad.lan.raisama.net>
	<jpgeg6wt794.fsf@linux.bootlegged.copy>
	<20160714205511.GA3727@thinpad.lan.raisama.net>
Date: Thu, 14 Jul 2016 17:02:40 -0400
In-Reply-To: <20160714205511.GA3727@thinpad.lan.raisama.net> (Eduardo
	Habkost's message of "Thu, 14 Jul 2016 17:55:11 -0300")
Message-ID: <jpg1t2wq6fj.fsf@linux.bootlegged.copy>
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [Qemu-devel] [PATCH v4 04/16] pc: forbid BSP removal
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel/>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Eduardo Habkost <ehabkost@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>, qemu-devel@nongnu.org, pkrempa@redhat.com, mst@redhat.com, eduardo.otubo@profitbricks.com

Eduardo Habkost <ehabkost@redhat.com> writes:
...
>> >>                                       DeviceState *dev, Error **errp)
>> >>  {
>> >> +    int idx;
>> >>      HotplugHandlerClass *hhc;
>> >>      Error *local_err = NULL;
>> >>      PCMachineState *pcms = PC_MACHINE(hotplug_dev);
>> >>  
>> >> +    pc_find_cpu_slot(pcms, CPU(dev), &idx);
>> >
>> > Looks fragile: if one day we create any TYPE_CPU object that is
>> > not in possible_cpus array, idx is undefined. I suggest
>> > initializing idx to -1 above.
>> 
>> Or just let pc_find_cpu_slot universally set it to -1 since
>> this series assumes that -1 means index isn't valid.
>
> I think it would be more intuitive if pc_find_cpu_slot() didn't
> touch *idx if no slot is found. But both ways sound good to me.

The caller then has to always take care of making sure there
is no bogus value in idx. Maybe, always calling if (pc_find_cpu_slot())
is better.