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@xxxxxxxxxx> --- 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
Attachment:
signature.asc
Description: PGP signature
