From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36943) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dV1bl-0006xq-UM for qemu-devel@nongnu.org; Tue, 11 Jul 2017 16:18:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dV1bi-0005p2-RF for qemu-devel@nongnu.org; Tue, 11 Jul 2017 16:18:09 -0400 Received: from mx1.redhat.com ([209.132.183.28]:44862) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dV1bi-0005op-Kv for qemu-devel@nongnu.org; Tue, 11 Jul 2017 16:18:06 -0400 Date: Tue, 11 Jul 2017 23:18:03 +0300 From: "Michael S. Tsirkin" Message-ID: <20170711231709-mutt-send-email-mst@kernel.org> References: <1499803333-9052-1-git-send-email-mark.cave-ayland@ilande.co.uk> <1499803333-9052-2-git-send-email-mark.cave-ayland@ilande.co.uk> <20170711201059.GA6020@localhost.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170711201059.GA6020@localhost.localdomain> Subject: Re: [Qemu-devel] [PATCHv8 1/3] fw_cfg: switch fw_cfg_find() to locate the fw_cfg device by type rather than path List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: Mark Cave-Ayland , qemu-devel@nongnu.org, lersek@redhat.com, somlo@cmu.edu, pbonzini@redhat.com, rjones@redhat.com, imammedo@redhat.com, peter.maydell@linaro.org On Tue, Jul 11, 2017 at 05:10:59PM -0300, Eduardo Habkost wrote: > On Tue, Jul 11, 2017 at 09:02:11PM +0100, Mark Cave-Ayland wrote: > > This will enable the fw_cfg device to be placed anywhere within the QOM tree > > regardless of its machine location. > > > > Note that we also add a comment to document the behaviour that we return NULL to > > indicate failure where either no fw_cfg device or multiple fw_cfg devices are > > found. > > > > Signed-off-by: Mark Cave-Ayland > > --- > > hw/nvram/fw_cfg.c | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c > > index 99bdbc2..8ef889a 100644 > > --- a/hw/nvram/fw_cfg.c > > +++ b/hw/nvram/fw_cfg.c > > @@ -1017,7 +1017,12 @@ FWCfgState *fw_cfg_init_mem(hwaddr ctl_addr, hwaddr data_addr) > > > > FWCfgState *fw_cfg_find(void) > > { > > - return FW_CFG(object_resolve_path(FW_CFG_PATH, NULL)); > > + /* Returns FWCfgState if only one fw_cfg device type exists. If zero or > > + more than one fw_cfg device are found then NULL is returned as per the > > + object_resolve_path_type() documentation. This behaviour is correct as > > + it ensures that we detect both missing fw_cfg devices and multiple > > + fw_cfg devices which could result in unpredictable behaviour. */ > > + return FW_CFG(object_resolve_path_type("", TYPE_FW_CFG, NULL)); > > } > > I believe a one-line "returns NULL unless there is exactly one > fw_cfg device" (similar to the one at find_vmgenid_dev()) would > suffice. :) Multi-line comments also should formatted /* * like * this */ not /* like * this */ > -- > Eduardo