Mailing List Archive

Re: [Qemu-devel] [QEMU PATCH] create struct for machine initialization arguments (v2)
On Sat, Oct 06, 2012 at 12:33:09AM +0400, Max Filippov wrote:
> On Sat, Oct 6, 2012 at 12:22 AM, Eduardo Habkost <ehabkost@redhat.com> wrote:
> > This should help us to:
> > - More easily add or remove machine initialization arguments without
> > having to change every single machine init function;
> > - More easily make mechanical changes involving the machine init
> > functions in the future;
> > - Let machine initialization forward the init arguments to other
> > functions more easily.
> >
> > This change was half-mechanical process: first the struct was added with
> > the local ram_size, boot_device, kernel_*, initrd_*, and cpu_model local
> > variable initialization to all functions. Then the compiler helped me
> > locate the local variables that are unused, so they could be removed.
> >
> > Changes v1 -> v2:
> > - Fix mistake on the conversion of pc_xen_hvm_init() and xen_init_pv()
> >
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>
> [...]
>
> > diff --git a/vl.c b/vl.c
> > index 8d305ca..f663e7c 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -3624,8 +3624,13 @@ int main(int argc, char **argv, char **envp)
> >
> > qdev_machine_init();
> >
> > - machine->init(ram_size, boot_devices,
> > - kernel_filename, kernel_cmdline, initrd_filename, cpu_model);
> > + QEMUMachineInitArgs args = { .ram_size = ram_size,
> > + .boot_device = boot_devices,
> > + .kernel_filename = kernel_filename,
> > + .kernel_cmdline = kernel_cmdline,
> > + initrd_filename = initrd_filename,
>
> Missing dot?

Funny, GCC didn't complain. Thanks for spotting it!

I am fixing this (and another problem I have found) and submit v3.

>
> > + .cpu_model = cpu_model };
> > + machine->init(&args);
> >
> > cpu_synchronize_all_post_init();
>
> --
> Thanks.
> -- Max
>

--
Eduardo

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Qemu-devel] [QEMU PATCH] create struct for machine initialization arguments (v2) [ In reply to ]
On 10/05/2012 02:40 PM, Eduardo Habkost wrote:

>>> - machine->init(ram_size, boot_devices,
>>> - kernel_filename, kernel_cmdline, initrd_filename, cpu_model);
>>> + QEMUMachineInitArgs args = { .ram_size = ram_size,
>>> + .boot_device = boot_devices,
>>> + .kernel_filename = kernel_filename,
>>> + .kernel_cmdline = kernel_cmdline,
>>> + initrd_filename = initrd_filename,
>>
>> Missing dot?
>
> Funny, GCC didn't complain. Thanks for spotting it!

Eww, insidious :P. This assigned local variable initrd_filename to
itself, then put the lvalue result of that assignment as the initializer
to the next available struct member residing after .kernel_cmdline
(which happened to be .initrd_filename). That is, gcc didn't complain
because it worked by sheer dumb luck.

--
Eric Blake eblake@redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org