On 11/07/17 21:10, 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 <mark.cave-ayl...@ilande.co.uk>
>> ---
>> 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. :)
I got the impression from the thread that Igor thought the semantics of
fw_cfg_find() weren't clear enough, so wanted to make sure a good
explanation was included in the patch.
I'll wait to see if there's any further feedback before a v9...
ATB,
Mark.
