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

[Peter Crosthwaite]

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.

Regards,
Peter