Peter Crosthwaite <peter.crosthwa...@xilinx.com> writes:
> On Wed, Apr 16, 2014 at 6:05 PM, Markus Armbruster <arm...@redhat.com> wrote:
>> Peter Crosthwaite <peter.crosthwa...@xilinx.com> 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.
