[PATCH 03/15] vbox: Drop @iid from UIMachine::LaunchVMProcess()

[Martin Kletzander]

On Tue, Jan 24, 2023 at 08:47:19AM +0100, Michal Prívozník wrote:
>On 1/23/23 11:00, Martin Kletzander wrote:
>> On Mon, Jan 23, 2023 at 10:34:12AM +0100, Michal Privoznik wrote:
>>> The @iid argument of UIMachine::LaunchVMProcess() callback is
>>> unused. Drop it and also its propagation from parent functions.
>>>
>>
>> Looks like even in 6.1 this was not a function parameter of
>> LaunchVMProcess.  Was it ever?
>
>Looks like it was. Commit v1.2.8-rc1~155 suggests it was used for VBox <
>4.0.
>
>> What I'm trying to figure out is whether
>> there is a way to make sure other cases don't happen or are not present
>> in the code already.
>
>I don't think there's an algorithmic way to check that. What I did in
>this series is drop all G_GNUC_UNUSED, let compiler warn me about unused
>arguments and then either put the flag back OR drop the argument if it's
>unused. My reasoning was that with ever-changing VBox API those unused
>arguments are leftovers from previous version of APIs (but I might be
>wrong).
>

I guess that's best effort we can provide for now.

>>
>>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>>> ---
>>> src/vbox/vbox_common.c        | 6 +++---
>>> src/vbox/vbox_tmpl.c          | 1 -
>>> src/vbox/vbox_uniformed_api.h | 1 -
>>> 3 files changed, 3 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
>>> index ea3a54b7c9..7b1b8bb1b0 100644
>>> --- a/src/vbox/vbox_common.c
>>> +++ b/src/vbox/vbox_common.c
>>> @@ -2046,7 +2046,7 @@ static int vboxDomainUndefine(virDomainPtr dom)
>>> }
>>>
>>> static int
>>> -vboxStartMachine(virDomainPtr dom, int maxDomID, IMachine *machine,
>>> vboxIID *iid)
>>> +vboxStartMachine(virDomainPtr dom, int maxDomID, IMachine *machine)
>>> {
>>>     struct _vboxDriver *data = dom->conn->privateData;
>>>     int vrdpPresent = 0;
>>> @@ -2147,7 +2147,7 @@ vboxStartMachine(virDomainPtr dom, int maxDomID,
>>> IMachine *machine, vboxIID *iid
>>>     if (vrdpPresent)
>>>         VBOX_UTF8_TO_UTF16("vrdp", &sessionType);
>>>
>>> -    rc = gVBoxAPI.UIMachine.LaunchVMProcess(data, machine, iid,
>>> +    rc = gVBoxAPI.UIMachine.LaunchVMProcess(data, machine,
>>>                                             sessionType, env,
>>>                                             &progress);
>>>
>>> @@ -2238,7 +2238,7 @@ static int
>>> vboxDomainCreateWithFlags(virDomainPtr dom, unsigned int flags)
>>>                 gVBoxAPI.UIMachine.GetState(machine, &state);
>>>
>>>                 if (gVBoxAPI.machineStateChecker.NotStart(state)) {
>>> -                    ret = vboxStartMachine(dom, i, machine, &iid);
>>> +                    ret = vboxStartMachine(dom, i, machine);
>>
>> Looks like this makes the iid unused in this function (or rather code
>> block) and can be removed too.
>
>I'm not so sure about that. There's a for() loop that traverses through
>all machines, trying to find the one that the function was called with
>and this is done by comparing iid (which in fact is domain UUID, not
>domain ID as we understand it).
>
>Or am I missing something?
>

Nope, I missed something:

vboxIIDToUUID(&iid, uuid);

this patch is fine then.

>Michal
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20230124/aee03275/attachment.sig>