Re: [Qemu-devel] [PATCH v1 0/3] Introduce qemu_get_boot_opts()

[Markus Armbruster]

Peter Crosthwaite <address@hidden> writes:

> On Wed, Apr 16, 2014 at 6:05 PM, Markus Armbruster <address@hidden> wrote:
>> Peter Crosthwaite <address@hidden> writes:
>>
>>> Hi Markus,
>>>
>>> This series introduces qemu_get_boot_opts(), in much the same way as
>>> was done for qemu_get_machine_opts().
>>>
>>> As usual, I have out-of-scope and out-of-tree usages :) But P3 does
>>> clean up the one existing instance of the long-and-awkward form of
>>> this query and makes it consistent with an immediately surrounding
>>> qemu_get_machine_opts().
>>
>> I doubt this is worthwhile on its own as it stands.
>>
>> However, you missed the two uses of "boot-opts" in hw/nvram/fw_cfg.c.
>> Since these uses are currently wrong the same way as the the uses of
>> "machine" fixed in commit 36ad0e9 were, covering them could strengthen
>> your case quite a bit,
>>
>
> Yeh I noticed it, andI thought that "get the first list element" code
> was weird. And I decided to let sleeing dogs lie. But are you saying
> its wrong and we can just simplify to:
>
> --- a/hw/nvram/fw_cfg.c
> +++ b/hw/nvram/fw_cfg.c
> @@ -125,18 +125,16 @@ static void fw_cfg_bootsplash(FWCfgState *s)
>      const char *temp;
>
>      /* get user configuration */
> -    QemuOptsList *plist = qemu_find_opts("boot-opts");
> -    QemuOpts *opts = QTAILQ_FIRST(&plist->head);
> +    QemuOpts *opts = qemu_get_boot_opts();
>
> ? Happy to make this change.

Also drop the if (opts != NULL).  Suggest to pattern the commit message
after commit 36ad0e9, or at least reference it.