[libvirt] [PATCH] vbox: Fix passing an empty IMedium* array to IMachine::Delete

vboxArray is not castable to a COM item type. vboxArray is a wrapper around the XPCOM and MSCOM specific array handling. In this case we can avoid passing NULL as an empty array to IMachine::Delete by passing a dummy IMedium* array with a single NULL item. --- Jean-Baptiste, I can not reproduce the assertion you mentioned, or I don't know where to look for it. So could you verify that is patch avoids this assertion? src/vbox/vbox_tmpl.c | 9 ++++----- 1 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 57c18a4..4b0ee2e 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -5294,11 +5294,10 @@ vboxDomainUndefineFlags(virDomainPtr dom, unsigned int flags) ((IMachine_Delete)machine->vtbl->Delete)(machine, &safeArray, &progress); # else - union { - vboxArray array; - IMedium *medium; - } u = { .array = VBOX_ARRAY_INITIALIZER }; - machine->vtbl->Delete(machine, 0, &u.medium, &progress); + /* XPCOM doesn't like NULL as an array, even when the array size is 0. + * Instead pass it a dummy array to avoid passing NULL. */ + IMedium *array[] = { NULL }; + machine->vtbl->Delete(machine, 0, array, &progress); # endif if (progress != NULL) { progress->vtbl->WaitForCompletion(progress, -1); -- 1.7.4.1

On Sunday 22 April 2012 10:35:59 Matthias Bolte wrote:
vboxArray is not castable to a COM item type. vboxArray is a wrapper around the XPCOM and MSCOM specific array handling.
In this case we can avoid passing NULL as an empty array to IMachine::Delete by passing a dummy IMedium* array with a single NULL item. ---
Jean-Baptiste, I can not reproduce the assertion you mentioned, or I don't know where to look for it. So could you verify that is patch avoids this assertion?
src/vbox/vbox_tmpl.c | 9 ++++----- 1 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 57c18a4..4b0ee2e 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -5294,11 +5294,10 @@ vboxDomainUndefineFlags(virDomainPtr dom, unsigned int flags)
((IMachine_Delete)machine->vtbl->Delete)(machine, &safeArray, &progress); # else - union { - vboxArray array; - IMedium *medium; - } u = { .array = VBOX_ARRAY_INITIALIZER }; - machine->vtbl->Delete(machine, 0, &u.medium, &progress); + /* XPCOM doesn't like NULL as an array, even when the array size is 0. + * Instead pass it a dummy array to avoid passing NULL. */ + IMedium *array[] = { NULL }; + machine->vtbl->Delete(machine, 0, array, &progress); # endif if (progress != NULL) { progress->vtbl->WaitForCompletion(progress, -1);
The patch is good, I don't get the assertion anymore. -- Jean-Baptiste ROUAULT Ingénieur R&D - diateam : Architectes de l'information Phone : +33 (0)2 98 050 050 Fax : +33 (0)2 98 050 051

On 04/23/2012 02:29 AM, Jean-Baptiste Rouault wrote:
On Sunday 22 April 2012 10:35:59 Matthias Bolte wrote:
vboxArray is not castable to a COM item type. vboxArray is a wrapper around the XPCOM and MSCOM specific array handling.
In this case we can avoid passing NULL as an empty array to IMachine::Delete by passing a dummy IMedium* array with a single NULL item. ---
Jean-Baptiste, I can not reproduce the assertion you mentioned, or I don't know where to look for it. So could you verify that is patch avoids this assertion?
+ /* XPCOM doesn't like NULL as an array, even when the array size is 0. + * Instead pass it a dummy array to avoid passing NULL. */ + IMedium *array[] = { NULL }; + machine->vtbl->Delete(machine, 0, array, &progress); # endif if (progress != NULL) { progress->vtbl->WaitForCompletion(progress, -1);
The patch is good, I don't get the assertion anymore.
Coupled with that functional test, I give my ACK to the code, as a much nicer solution than my temporary build-breaker "fix" via a union hack. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Am 23. April 2012 18:48 schrieb Eric Blake <eblake@redhat.com>:
On 04/23/2012 02:29 AM, Jean-Baptiste Rouault wrote:
On Sunday 22 April 2012 10:35:59 Matthias Bolte wrote:
vboxArray is not castable to a COM item type. vboxArray is a wrapper around the XPCOM and MSCOM specific array handling.
In this case we can avoid passing NULL as an empty array to IMachine::Delete by passing a dummy IMedium* array with a single NULL item. ---
Jean-Baptiste, I can not reproduce the assertion you mentioned, or I don't know where to look for it. So could you verify that is patch avoids this assertion?
+ /* XPCOM doesn't like NULL as an array, even when the array size is 0. + * Instead pass it a dummy array to avoid passing NULL. */ + IMedium *array[] = { NULL }; + machine->vtbl->Delete(machine, 0, array, &progress); # endif if (progress != NULL) { progress->vtbl->WaitForCompletion(progress, -1);
The patch is good, I don't get the assertion anymore.
Coupled with that functional test, I give my ACK to the code, as a much nicer solution than my temporary build-breaker "fix" via a union hack.
Thanks, pushed. -- Matthias Bolte http://photron.blogspot.com
participants (3)
-
Eric Blake
-
Jean-Baptiste Rouault
-
Matthias Bolte