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.
Regards,
Peter
