[PATCH 00/13] vsh: Fix handling of commands and help - part 3 (commandline parser overhaul)

This series refactors the commandline parser in order to use easier to understand/maintain logic. Peter Krempa (13): meson: tests: Add 'virsh' as dependency of 'virshtest' tools: Rename vshCommandOptStringReq to vshCommandOptString vsh: Fix 'stdin' closing in 'cmdComplete' vsh: Add a VSH_OT_STRING argument for 'virsh echo' virshtest: Add test cases for command completion helper vsh: Rework logic for picking which argument is to be completed virsh: Introduce new 'VSH_OT_ARGV' accessors vsh: Remove unused infrastructure for command completion vsh: Unexport command lookup helpers 'vshCmddefSearch', 'vshCmdGrpSearch', 'vshCmdGrpHelp' vsh: Refactor parsed option and command assignment vshCmddefCheckInternals: Remove check for "too many options" vsh: Move option assignment debugging from vshCommandParse to vshCmdOptAssign vsh: Refactor logic in vshCommandParse tests/meson.build | 4 +- tests/virshtest.c | 37 + .../completion-arg-full-argv-next.out | 2 + .../completion-arg-full-argv.out | 2 + .../completion-arg-full-bool-next.out | 7 + .../completion-arg-full-bool.out | 2 + .../completion-arg-full-string-next.out | 7 + .../completion-arg-full-string.out | 2 + .../virshtestdata/completion-arg-partial.out | 4 + .../completion-arg-positional-empty.out | 3 + ...completion-arg-positional-partial-next.out | 2 + .../completion-arg-positional-partial.out | 2 + tests/virshtestdata/completion-args.out | 7 + .../completion-argv-multiple-next.out | 25 + ...mpletion-argv-multiple-positional-next.out | 25 + .../completion-argv-multiple-positional.out | 2 + .../completion-argv-multiple.out | 2 + .../completion-command-complete.out | 2 + tests/virshtestdata/completion-command.out | 2 + tests/virshtestdata/completion.in | 16 + tests/virshtestdata/completion.out | 71 ++ tools/virsh-backup.c | 4 +- tools/virsh-checkpoint.c | 18 +- tools/virsh-completer-domain.c | 2 +- tools/virsh-completer-host.c | 8 +- tools/virsh-domain-event.c | 2 +- tools/virsh-domain-monitor.c | 18 +- tools/virsh-domain.c | 351 +++--- tools/virsh-host.c | 40 +- tools/virsh-interface.c | 6 +- tools/virsh-network.c | 35 +- tools/virsh-nodedev.c | 30 +- tools/virsh-nwfilter.c | 8 +- tools/virsh-pool.c | 64 +- tools/virsh-secret.c | 10 +- tools/virsh-snapshot.c | 23 +- tools/virsh-util.c | 2 +- tools/virsh-volume.c | 29 +- tools/virsh.c | 6 +- tools/virt-admin.c | 26 +- tools/vsh.c | 1012 +++++++++-------- tools/vsh.h | 27 +- 42 files changed, 1103 insertions(+), 844 deletions(-) create mode 100644 tests/virshtestdata/completion-arg-full-argv-next.out create mode 100644 tests/virshtestdata/completion-arg-full-argv.out create mode 100644 tests/virshtestdata/completion-arg-full-bool-next.out create mode 100644 tests/virshtestdata/completion-arg-full-bool.out create mode 100644 tests/virshtestdata/completion-arg-full-string-next.out create mode 100644 tests/virshtestdata/completion-arg-full-string.out create mode 100644 tests/virshtestdata/completion-arg-partial.out create mode 100644 tests/virshtestdata/completion-arg-positional-empty.out create mode 100644 tests/virshtestdata/completion-arg-positional-partial-next.out create mode 100644 tests/virshtestdata/completion-arg-positional-partial.out create mode 100644 tests/virshtestdata/completion-args.out create mode 100644 tests/virshtestdata/completion-argv-multiple-next.out create mode 100644 tests/virshtestdata/completion-argv-multiple-positional-next.out create mode 100644 tests/virshtestdata/completion-argv-multiple-positional.out create mode 100644 tests/virshtestdata/completion-argv-multiple.out create mode 100644 tests/virshtestdata/completion-command-complete.out create mode 100644 tests/virshtestdata/completion-command.out create mode 100644 tests/virshtestdata/completion.in create mode 100644 tests/virshtestdata/completion.out -- 2.44.0

Ensure that virsh is rebuilt if needed. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/meson.build | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/meson.build b/tests/meson.build index 3f8f3dee7c..7270840428 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -298,7 +298,7 @@ tests += [ { 'name': 'virportallocatortest' }, { 'name': 'virrotatingfiletest' }, { 'name': 'virschematest' }, - { 'name': 'virshtest' }, + { 'name': 'virshtest', 'depends': [ virsh_prog ] }, { 'name': 'virstringtest' }, { 'name': 'virsystemdtest' }, { 'name': 'virtimetest' }, @@ -602,7 +602,7 @@ foreach data : tests test_bin, env: tests_env, timeout: data.get('timeout', 30), - depends: tests_deps, + depends: tests_deps + data.get('depends', []), suite: 'bin' ) endforeach -- 2.44.0

Shorten the function name as there isn't any vshCommandOptString. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/virsh-backup.c | 4 +- tools/virsh-checkpoint.c | 8 +- tools/virsh-completer-domain.c | 2 +- tools/virsh-completer-host.c | 8 +- tools/virsh-domain-event.c | 2 +- tools/virsh-domain-monitor.c | 10 +- tools/virsh-domain.c | 248 ++++++++++++++++----------------- tools/virsh-host.c | 40 +++--- tools/virsh-interface.c | 6 +- tools/virsh-network.c | 26 ++-- tools/virsh-nodedev.c | 30 ++-- tools/virsh-nwfilter.c | 8 +- tools/virsh-pool.c | 64 ++++----- tools/virsh-secret.c | 10 +- tools/virsh-snapshot.c | 14 +- tools/virsh-util.c | 2 +- tools/virsh-volume.c | 29 ++-- tools/virsh.c | 2 +- tools/virt-admin.c | 22 +-- tools/vsh.c | 10 +- tools/vsh.h | 2 +- 21 files changed, 273 insertions(+), 274 deletions(-) diff --git a/tools/virsh-backup.c b/tools/virsh-backup.c index 9a1e89760f..27777eea74 100644 --- a/tools/virsh-backup.c +++ b/tools/virsh-backup.c @@ -68,7 +68,7 @@ cmdBackupBegin(vshControl *ctl, if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptStringReq(ctl, cmd, "backupxml", &backup_from) < 0) + if (vshCommandOptString(ctl, cmd, "backupxml", &backup_from) < 0) return false; if (!backup_from) { @@ -80,7 +80,7 @@ cmdBackupBegin(vshControl *ctl, } } - if (vshCommandOptStringReq(ctl, cmd, "checkpointxml", &check_from) < 0) + if (vshCommandOptString(ctl, cmd, "checkpointxml", &check_from) < 0) return false; if (check_from) { if (virFileReadAll(check_from, VSH_MAX_XML_FILE, &check_buffer) < 0) { diff --git a/tools/virsh-checkpoint.c b/tools/virsh-checkpoint.c index e3fd6b2df2..fea6b4fb4b 100644 --- a/tools/virsh-checkpoint.c +++ b/tools/virsh-checkpoint.c @@ -121,7 +121,7 @@ cmdCheckpointCreate(vshControl *ctl, if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptStringReq(ctl, cmd, "xmlfile", &from) < 0) + if (vshCommandOptString(ctl, cmd, "xmlfile", &from) < 0) return false; if (!from) { buffer = g_strdup("<domaincheckpoint/>"); @@ -234,8 +234,8 @@ cmdCheckpointCreateAs(vshControl *ctl, if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptStringReq(ctl, cmd, "name", &name) < 0 || - vshCommandOptStringReq(ctl, cmd, "description", &desc) < 0) + if (vshCommandOptString(ctl, cmd, "name", &name) < 0 || + vshCommandOptString(ctl, cmd, "description", &desc) < 0) return false; virBufferAddLit(&buf, "<domaincheckpoint>\n"); @@ -281,7 +281,7 @@ virshLookupCheckpoint(vshControl *ctl, { const char *chkname = NULL; - if (vshCommandOptStringReq(ctl, cmd, arg, &chkname) < 0) + if (vshCommandOptString(ctl, cmd, arg, &chkname) < 0) return -1; if (!(*chk = virDomainCheckpointLookupByName(dom, chkname, 0))) diff --git a/tools/virsh-completer-domain.c b/tools/virsh-completer-domain.c index 2891d1399c..61362224a3 100644 --- a/tools/virsh-completer-domain.c +++ b/tools/virsh-completer-domain.c @@ -381,7 +381,7 @@ virshDomainInterfaceStateCompleter(vshControl *ctl, if (virshDomainGetXML(ctl, cmd, flags, &xml, &ctxt) < 0) return NULL; - if (vshCommandOptStringReq(ctl, cmd, "interface", &iface) < 0) + if (vshCommandOptString(ctl, cmd, "interface", &iface) < 0) return NULL; /* normalize the mac addr */ diff --git a/tools/virsh-completer-host.c b/tools/virsh-completer-host.c index 93b633eb64..78d2236f97 100644 --- a/tools/virsh-completer-host.c +++ b/tools/virsh-completer-host.c @@ -227,10 +227,10 @@ virshCPUModelCompleter(vshControl *ctl, virCheckFlags(0, NULL); - if (vshCommandOptStringReq(ctl, cmd, "virttype", &virttype) < 0 || - vshCommandOptStringReq(ctl, cmd, "emulator", &emulator) < 0 || - vshCommandOptStringReq(ctl, cmd, "arch", &arch) < 0 || - vshCommandOptStringReq(ctl, cmd, "machine", &machine) < 0) + if (vshCommandOptString(ctl, cmd, "virttype", &virttype) < 0 || + vshCommandOptString(ctl, cmd, "emulator", &emulator) < 0 || + vshCommandOptString(ctl, cmd, "arch", &arch) < 0 || + vshCommandOptString(ctl, cmd, "machine", &machine) < 0) return NULL; if (!priv->conn || virConnectIsAlive(priv->conn) <= 0) diff --git a/tools/virsh-domain-event.c b/tools/virsh-domain-event.c index 8bf57ade7a..cd33d4d938 100644 --- a/tools/virsh-domain-event.c +++ b/tools/virsh-domain-event.c @@ -927,7 +927,7 @@ cmdEvent(vshControl *ctl, const vshCmd *cmd) return true; } - if (vshCommandOptStringReq(ctl, cmd, "event", &eventName) < 0) + if (vshCommandOptString(ctl, cmd, "event", &eventName) < 0) return false; if (!eventName && !all) { diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index 599ae71e7a..700f3ae094 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -765,7 +765,7 @@ cmdDomIfGetLink(vshControl *ctl, const vshCmd *cmd) int ninterfaces; unsigned int flags = 0; - if (vshCommandOptStringReq(ctl, cmd, "interface", &iface) < 0) + if (vshCommandOptString(ctl, cmd, "interface", &iface) < 0) return false; if (vshCommandOptBool(cmd, "config")) @@ -934,7 +934,7 @@ cmdDomblkstat(vshControl *ctl, const vshCmd *cmd) string to denote 'all devices'. A NULL device arg would violate API contract. */ - if (vshCommandOptStringReq(ctl, cmd, "device", &device) < 0) + if (vshCommandOptString(ctl, cmd, "device", &device) < 0) return false; if (!device) @@ -1058,7 +1058,7 @@ cmdDomIfstat(vshControl *ctl, const vshCmd *cmd) if (!(dom = virshCommandOptDomain(ctl, cmd, &name))) return false; - if (vshCommandOptStringReq(ctl, cmd, "interface", &device) < 0) + if (vshCommandOptString(ctl, cmd, "interface", &device) < 0) return false; if (virDomainInterfaceStats(dom, device, &stats, sizeof(stats)) == -1) { @@ -2254,9 +2254,9 @@ cmdDomIfAddr(vshControl *ctl, const vshCmd *cmd) const char *sourcestr = NULL; int source = VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_LEASE; - if (vshCommandOptStringReq(ctl, cmd, "interface", &ifacestr) < 0) + if (vshCommandOptString(ctl, cmd, "interface", &ifacestr) < 0) return false; - if (vshCommandOptStringReq(ctl, cmd, "source", &sourcestr) < 0) + if (vshCommandOptString(ctl, cmd, "source", &sourcestr) < 0) return false; if (sourcestr && diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 3d9c48629a..28d90377a1 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -382,7 +382,7 @@ cmdAttachDevice(vshControl *ctl, const vshCmd *cmd) if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) + if (vshCommandOptString(ctl, cmd, "file", &from) < 0) return false; if (persistent && @@ -647,25 +647,25 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) if (live) flags |= VIR_DOMAIN_AFFECT_LIVE; - if (vshCommandOptStringReq(ctl, cmd, "source", &source) < 0 || - vshCommandOptStringReq(ctl, cmd, "target", &target) < 0 || - vshCommandOptStringReq(ctl, cmd, "driver", &driver) < 0 || - vshCommandOptStringReq(ctl, cmd, "subdriver", &subdriver) < 0 || - vshCommandOptStringReq(ctl, cmd, "type", &device) < 0 || - vshCommandOptStringReq(ctl, cmd, "mode", &mode) < 0 || - vshCommandOptStringReq(ctl, cmd, "iothread", &iothread) < 0 || - vshCommandOptStringReq(ctl, cmd, "cache", &cache) < 0 || - vshCommandOptStringReq(ctl, cmd, "io", &io) < 0 || - vshCommandOptStringReq(ctl, cmd, "serial", &serial) < 0 || - vshCommandOptStringReq(ctl, cmd, "wwn", &wwn) < 0 || - vshCommandOptStringReq(ctl, cmd, "address", &straddr) < 0 || - vshCommandOptStringReq(ctl, cmd, "targetbus", &targetbus) < 0 || - vshCommandOptStringReq(ctl, cmd, "alias", &alias) < 0 || - vshCommandOptStringReq(ctl, cmd, "sourcetype", &stype) < 0 || - vshCommandOptStringReq(ctl, cmd, "source-protocol", &source_protocol) < 0 || - vshCommandOptStringReq(ctl, cmd, "source-host-name", &host_name) < 0 || - vshCommandOptStringReq(ctl, cmd, "source-host-transport", &host_transport) < 0 || - vshCommandOptStringReq(ctl, cmd, "source-host-socket", &host_socket) < 0) + if (vshCommandOptString(ctl, cmd, "source", &source) < 0 || + vshCommandOptString(ctl, cmd, "target", &target) < 0 || + vshCommandOptString(ctl, cmd, "driver", &driver) < 0 || + vshCommandOptString(ctl, cmd, "subdriver", &subdriver) < 0 || + vshCommandOptString(ctl, cmd, "type", &device) < 0 || + vshCommandOptString(ctl, cmd, "mode", &mode) < 0 || + vshCommandOptString(ctl, cmd, "iothread", &iothread) < 0 || + vshCommandOptString(ctl, cmd, "cache", &cache) < 0 || + vshCommandOptString(ctl, cmd, "io", &io) < 0 || + vshCommandOptString(ctl, cmd, "serial", &serial) < 0 || + vshCommandOptString(ctl, cmd, "wwn", &wwn) < 0 || + vshCommandOptString(ctl, cmd, "address", &straddr) < 0 || + vshCommandOptString(ctl, cmd, "targetbus", &targetbus) < 0 || + vshCommandOptString(ctl, cmd, "alias", &alias) < 0 || + vshCommandOptString(ctl, cmd, "sourcetype", &stype) < 0 || + vshCommandOptString(ctl, cmd, "source-protocol", &source_protocol) < 0 || + vshCommandOptString(ctl, cmd, "source-host-name", &host_name) < 0 || + vshCommandOptString(ctl, cmd, "source-host-transport", &host_transport) < 0 || + vshCommandOptString(ctl, cmd, "source-host-socket", &host_socket) < 0) return false; if (stype && @@ -963,16 +963,16 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd) if (live) flags |= VIR_DOMAIN_AFFECT_LIVE; - if (vshCommandOptStringReq(ctl, cmd, "type", &type) < 0 || - vshCommandOptStringReq(ctl, cmd, "source", &source) < 0 || - vshCommandOptStringReq(ctl, cmd, "target", &target) < 0 || - vshCommandOptStringReq(ctl, cmd, "mac", &mac) < 0 || - vshCommandOptStringReq(ctl, cmd, "script", &script) < 0 || - vshCommandOptStringReq(ctl, cmd, "model", &model) < 0 || - vshCommandOptStringReq(ctl, cmd, "alias", &alias) < 0 || - vshCommandOptStringReq(ctl, cmd, "inbound", &inboundStr) < 0 || - vshCommandOptStringReq(ctl, cmd, "outbound", &outboundStr) < 0 || - vshCommandOptStringReq(ctl, cmd, "source-mode", &sourceModeStr) < 0) + if (vshCommandOptString(ctl, cmd, "type", &type) < 0 || + vshCommandOptString(ctl, cmd, "source", &source) < 0 || + vshCommandOptString(ctl, cmd, "target", &target) < 0 || + vshCommandOptString(ctl, cmd, "mac", &mac) < 0 || + vshCommandOptString(ctl, cmd, "script", &script) < 0 || + vshCommandOptString(ctl, cmd, "model", &model) < 0 || + vshCommandOptString(ctl, cmd, "alias", &alias) < 0 || + vshCommandOptString(ctl, cmd, "inbound", &inboundStr) < 0 || + vshCommandOptString(ctl, cmd, "outbound", &outboundStr) < 0 || + vshCommandOptString(ctl, cmd, "source-mode", &sourceModeStr) < 0) return false; /* check interface type */ @@ -1402,7 +1402,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) if (!(dom = virshCommandOptDomain(ctl, cmd, &name))) goto cleanup; - if (vshCommandOptStringReq(ctl, cmd, "device", &disk) < 0) + if (vshCommandOptString(ctl, cmd, "device", &disk) < 0) goto cleanup; #define VSH_ADD_IOTUNE_SCALED(PARAM, CONST) \ @@ -1450,7 +1450,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) VSH_ADD_IOTUNE(write-iops-sec-max-length, WRITE_IOPS_SEC_MAX_LENGTH); #undef VSH_ADD_IOTUNE - if (vshCommandOptStringReq(ctl, cmd, "group-name", &group_name) < 0) { + if (vshCommandOptString(ctl, cmd, "group-name", &group_name) < 0) { vshError(ctl, "%s", _("Unable to parse group-name parameter")); goto cleanup; } @@ -2095,13 +2095,13 @@ cmdBlockcommit(vshControl *ctl, const vshCmd *cmd) VSH_EXCLUSIVE_OPTIONS("pivot", "keep-overlay"); - if (vshCommandOptStringReq(ctl, cmd, "path", &path) < 0) + if (vshCommandOptString(ctl, cmd, "path", &path) < 0) return false; - if (vshCommandOptStringReq(ctl, cmd, "base", &base) < 0) + if (vshCommandOptString(ctl, cmd, "base", &base) < 0) return false; - if (vshCommandOptStringReq(ctl, cmd, "top", &top) < 0) + if (vshCommandOptString(ctl, cmd, "top", &top) < 0) return false; if (vshBlockJobOptionBandwidth(ctl, cmd, bytes, &bandwidth) < 0) @@ -2352,13 +2352,13 @@ cmdBlockcopy(vshControl *ctl, const vshCmd *cmd) virshBlockJobWaitData *bjWait = NULL; int nparams = 0; - if (vshCommandOptStringReq(ctl, cmd, "path", &path) < 0) + if (vshCommandOptString(ctl, cmd, "path", &path) < 0) return false; - if (vshCommandOptStringReq(ctl, cmd, "dest", &dest) < 0) + if (vshCommandOptString(ctl, cmd, "dest", &dest) < 0) return false; - if (vshCommandOptStringReq(ctl, cmd, "xml", &xml) < 0) + if (vshCommandOptString(ctl, cmd, "xml", &xml) < 0) return false; - if (vshCommandOptStringReq(ctl, cmd, "format", &format) < 0) + if (vshCommandOptString(ctl, cmd, "format", &format) < 0) return false; if (vshBlockJobOptionBandwidth(ctl, cmd, bytes, &bandwidth) < 0) return false; @@ -2755,7 +2755,7 @@ cmdBlockjob(vshControl *ctl, const vshCmd *cmd) return false; /* XXX Allow path to be optional to list info on all devices at once */ - if (vshCommandOptStringReq(ctl, cmd, "path", &path) < 0) + if (vshCommandOptString(ctl, cmd, "path", &path) < 0) return false; if (bandwidth) @@ -2840,10 +2840,10 @@ cmdBlockpull(vshControl *ctl, const vshCmd *cmd) VSH_REQUIRE_OPTION("verbose", "wait"); VSH_REQUIRE_OPTION("async", "wait"); - if (vshCommandOptStringReq(ctl, cmd, "path", &path) < 0) + if (vshCommandOptString(ctl, cmd, "path", &path) < 0) return false; - if (vshCommandOptStringReq(ctl, cmd, "base", &base) < 0) + if (vshCommandOptString(ctl, cmd, "base", &base) < 0) return false; if (vshBlockJobOptionBandwidth(ctl, cmd, bytes, &bandwidth) < 0) @@ -2950,7 +2950,7 @@ cmdBlockresize(vshControl *ctl, const vshCmd *cmd) VSH_ALTERNATIVE_OPTIONS("size", "capacity"); - if (vshCommandOptStringReq(ctl, cmd, "path", (const char **) &path) < 0) + if (vshCommandOptString(ctl, cmd, "path", (const char **) &path) < 0) return false; if (vshCommandOptScaledInt(ctl, cmd, "size", &size, 1024, ULLONG_MAX) < 0) @@ -3052,7 +3052,7 @@ cmdConsole(vshControl *ctl, const vshCmd *cmd) if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptStringReq(ctl, cmd, "devname", &name) < 0) /* sc_prohibit_devname */ + if (vshCommandOptString(ctl, cmd, "devname", &name) < 0) /* sc_prohibit_devname */ return false; if (force) @@ -3121,8 +3121,8 @@ cmdDomIfSetLink(vshControl *ctl, const vshCmd *cmd) if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptStringReq(ctl, cmd, "interface", &iface) < 0 || - vshCommandOptStringReq(ctl, cmd, "state", &state) < 0) + if (vshCommandOptString(ctl, cmd, "interface", &iface) < 0 || + vshCommandOptString(ctl, cmd, "state", &state) < 0) return false; if (STRNEQ(state, "up") && STRNEQ(state, "down")) { @@ -3276,11 +3276,11 @@ cmdDomIftune(vshControl *ctl, const vshCmd *cmd) if (!(dom = virshCommandOptDomain(ctl, cmd, &name))) return false; - if (vshCommandOptStringReq(ctl, cmd, "interface", &device) < 0) + if (vshCommandOptString(ctl, cmd, "interface", &device) < 0) goto cleanup; - if (vshCommandOptStringReq(ctl, cmd, "inbound", &inboundStr) < 0 || - vshCommandOptStringReq(ctl, cmd, "outbound", &outboundStr) < 0) + if (vshCommandOptString(ctl, cmd, "inbound", &inboundStr) < 0 || + vshCommandOptString(ctl, cmd, "outbound", &outboundStr) < 0) goto cleanup; if (inboundStr) { @@ -3481,7 +3481,7 @@ cmdDomPMSuspend(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptULongLong(ctl, cmd, "duration", &duration) < 0) return false; - if (vshCommandOptStringReq(ctl, cmd, "target", &target) < 0) + if (vshCommandOptString(ctl, cmd, "target", &target) < 0) return false; if ((suspendTarget = virshNodeSuspendTargetTypeFromString(target)) < 0) { @@ -4188,7 +4188,7 @@ doSave(void *opaque) goto out_sig; #endif /* !WIN32 */ - if (vshCommandOptStringReq(ctl, cmd, "file", &to) < 0) + if (vshCommandOptString(ctl, cmd, "file", &to) < 0) goto out; if (vshCommandOptBool(cmd, "bypass-cache")) @@ -4198,7 +4198,7 @@ doSave(void *opaque) if (vshCommandOptBool(cmd, "paused")) flags |= VIR_DOMAIN_SAVE_PAUSED; - if (vshCommandOptStringReq(ctl, cmd, "xml", &xmlfile) < 0) + if (vshCommandOptString(ctl, cmd, "xml", &xmlfile) < 0) goto out; if (!(dom = virshCommandOptDomain(ctl, cmd, &name))) @@ -4454,7 +4454,7 @@ cmdSave(vshControl *ctl, const vshCmd *cmd) if (!(dom = virshCommandOptDomain(ctl, cmd, &name))) return false; - if (vshCommandOptStringReq(ctl, cmd, "file", &to) < 0) + if (vshCommandOptString(ctl, cmd, "file", &to) < 0) return false; if (vshCommandOptBool(cmd, "verbose")) @@ -4521,7 +4521,7 @@ cmdSaveImageDumpxml(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool(cmd, "security-info")) flags |= VIR_DOMAIN_XML_SECURE; - if (vshCommandOptStringReq(ctl, cmd, "file", &file) < 0) + if (vshCommandOptString(ctl, cmd, "file", &file) < 0) return false; if (vshCommandOptStringQuiet(ctl, cmd, "xpath", &xpath) < 0) @@ -4581,10 +4581,10 @@ cmdSaveImageDefine(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool(cmd, "paused")) flags |= VIR_DOMAIN_SAVE_PAUSED; - if (vshCommandOptStringReq(ctl, cmd, "file", &file) < 0) + if (vshCommandOptString(ctl, cmd, "file", &file) < 0) return false; - if (vshCommandOptStringReq(ctl, cmd, "xml", &xmlfile) < 0) + if (vshCommandOptString(ctl, cmd, "xml", &xmlfile) < 0) return false; if (virFileReadAll(xmlfile, VSH_MAX_XML_FILE, &xml) < 0) @@ -4645,7 +4645,7 @@ cmdSaveImageEdit(vshControl *ctl, const vshCmd *cmd) * flags, so we reject it up front to avoid looping. */ VSH_EXCLUSIVE_OPTIONS("running", "paused"); - if (vshCommandOptStringReq(ctl, cmd, "file", &file) < 0) + if (vshCommandOptString(ctl, cmd, "file", &file) < 0) return false; #define EDIT_GET_XML \ @@ -4980,7 +4980,7 @@ cmdManagedSaveDefine(vshControl *ctl, const vshCmd *cmd) VSH_EXCLUSIVE_OPTIONS("running", "paused"); - if (vshCommandOptStringReq(ctl, cmd, "xml", &xmlfile) < 0) + if (vshCommandOptString(ctl, cmd, "xml", &xmlfile) < 0) return false; if (virFileReadAll(xmlfile, VSH_MAX_XML_FILE, &xml) < 0) @@ -5089,7 +5089,7 @@ cmdSchedInfoUpdate(vshControl *ctl, const vshCmd *cmd, goto cleanup; } - rv = vshCommandOptStringReq(ctl, cmd, "cap", &val); + rv = vshCommandOptString(ctl, cmd, "cap", &val); if (rv < 0 || (val && cmdSchedInfoUpdateOne(ctl, src_params, nsrc_params, @@ -5097,7 +5097,7 @@ cmdSchedInfoUpdate(vshControl *ctl, const vshCmd *cmd, "cap", val) < 0)) goto cleanup; - rv = vshCommandOptStringReq(ctl, cmd, "weight", &val); + rv = vshCommandOptString(ctl, cmd, "weight", &val); if (rv < 0 || (val && cmdSchedInfoUpdateOne(ctl, src_params, nsrc_params, @@ -5263,7 +5263,7 @@ cmdRestore(vshControl *ctl, const vshCmd *cmd) virshControl *priv = ctl->privData; int rc; - if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) + if (vshCommandOptString(ctl, cmd, "file", &from) < 0) return false; if (vshCommandOptBool(cmd, "bypass-cache")) @@ -5275,7 +5275,7 @@ cmdRestore(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool(cmd, "reset-nvram")) flags |= VIR_DOMAIN_SAVE_RESET_NVRAM; - if (vshCommandOptStringReq(ctl, cmd, "xml", &xmlfile) < 0) + if (vshCommandOptString(ctl, cmd, "xml", &xmlfile) < 0) return false; if (xmlfile && @@ -5372,7 +5372,7 @@ doDump(void *opaque) goto out_sig; #endif /* !WIN32 */ - if (vshCommandOptStringReq(ctl, cmd, "file", &to) < 0) + if (vshCommandOptString(ctl, cmd, "file", &to) < 0) goto out; if (!(dom = virshCommandOptDomain(ctl, cmd, &name))) @@ -5445,7 +5445,7 @@ cmdDump(vshControl *ctl, const vshCmd *cmd) if (!(dom = virshCommandOptDomain(ctl, cmd, &name))) return false; - if (vshCommandOptStringReq(ctl, cmd, "file", &to) < 0) + if (vshCommandOptString(ctl, cmd, "file", &to) < 0) return false; if (vshCommandOptBool(cmd, "verbose")) @@ -5536,7 +5536,7 @@ cmdScreenshot(vshControl *ctl, const vshCmd *cmd) virshControl *priv = ctl->privData; virshStreamCallbackData cbdata; - if (vshCommandOptStringReq(ctl, cmd, "file", (const char **) &file) < 0) + if (vshCommandOptString(ctl, cmd, "file", (const char **) &file) < 0) return false; if (vshCommandOptUInt(ctl, cmd, "screen", &screen) < 0) @@ -5666,8 +5666,8 @@ cmdSetLifecycleAction(vshControl *ctl, const vshCmd *cmd) if (live) flags |= VIR_DOMAIN_AFFECT_LIVE; - if (vshCommandOptStringReq(ctl, cmd, "type", &typeStr) < 0 || - vshCommandOptStringReq(ctl, cmd, "action", &actionStr) < 0) { + if (vshCommandOptString(ctl, cmd, "type", &typeStr) < 0 || + vshCommandOptString(ctl, cmd, "action", &actionStr) < 0) { return false; } @@ -5735,10 +5735,10 @@ cmdSetUserPassword(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool(cmd, "encrypted")) flags = VIR_DOMAIN_PASSWORD_ENCRYPTED; - if (vshCommandOptStringReq(ctl, cmd, "user", &user) < 0) + if (vshCommandOptString(ctl, cmd, "user", &user) < 0) return false; - if (vshCommandOptStringReq(ctl, cmd, "password", &password) < 0) + if (vshCommandOptString(ctl, cmd, "password", &password) < 0) return false; if (!(dom = virshCommandOptDomain(ctl, cmd, &name))) @@ -5811,7 +5811,7 @@ cmdShutdown(vshControl *ctl, const vshCmd *cmd) g_auto(GStrv) modes = NULL; char **tmp; - if (vshCommandOptStringReq(ctl, cmd, "mode", &mode) < 0) + if (vshCommandOptString(ctl, cmd, "mode", &mode) < 0) return false; if (mode && !(modes = g_strsplit(mode, ",", 0))) { @@ -5886,7 +5886,7 @@ cmdReboot(vshControl *ctl, const vshCmd *cmd) g_auto(GStrv) modes = NULL; char **tmp; - if (vshCommandOptStringReq(ctl, cmd, "mode", &mode) < 0) + if (vshCommandOptString(ctl, cmd, "mode", &mode) < 0) return false; if (mode && !(modes = g_strsplit(mode, ",", 0))) { @@ -6982,7 +6982,7 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd) if (live) flags |= VIR_DOMAIN_AFFECT_LIVE; - if (vshCommandOptStringReq(ctl, cmd, "cpulist", &cpulist) < 0) + if (vshCommandOptString(ctl, cmd, "cpulist", &cpulist) < 0) return false; if (!cpulist) @@ -7076,7 +7076,7 @@ cmdEmulatorPin(vshControl *ctl, const vshCmd *cmd) if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptStringReq(ctl, cmd, "cpulist", &cpulist) < 0) { + if (vshCommandOptString(ctl, cmd, "cpulist", &cpulist) < 0) { return false; } query = !cpulist; @@ -7253,7 +7253,7 @@ cmdGuestvcpus(vshControl *ctl, const vshCmd *cmd) VSH_REQUIRE_OPTION("enable", "cpulist"); VSH_REQUIRE_OPTION("disable", "cpulist"); - if (vshCommandOptStringReq(ctl, cmd, "cpulist", &cpulist)) + if (vshCommandOptString(ctl, cmd, "cpulist", &cpulist)) return false; if (cpulist && !(enable || disable)) { @@ -7346,7 +7346,7 @@ cmdSetvcpu(vshControl *ctl, const vshCmd *cmd) return false; } - if (vshCommandOptStringReq(ctl, cmd, "vcpulist", &vcpulist)) + if (vshCommandOptString(ctl, cmd, "vcpulist", &vcpulist)) return false; if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) @@ -7396,7 +7396,7 @@ cmdDomblkthreshold(vshControl *ctl, const vshCmd *cmd) const char *dev = NULL; g_autoptr(virshDomain) dom = NULL; - if (vshCommandOptStringReq(ctl, cmd, "dev", &dev)) + if (vshCommandOptString(ctl, cmd, "dev", &dev)) return false; if (vshCommandOptScaledInt(ctl, cmd, "threshold", @@ -7552,7 +7552,7 @@ cmdIOThreadPin(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptUInt(ctl, cmd, "iothread", &iothread_id) < 0) return false; - if (vshCommandOptStringReq(ctl, cmd, "cpulist", &cpulist) < 0) + if (vshCommandOptString(ctl, cmd, "cpulist", &cpulist) < 0) return false; if ((maxcpu = virshNodeGetCPUCount(priv->conn)) < 0) @@ -8052,7 +8052,7 @@ cmdCreate(vshControl *ctl, const vshCmd *cmd) g_autofree int *fds = NULL; virshControl *priv = ctl->privData; - if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) + if (vshCommandOptString(ctl, cmd, "file", &from) < 0) return false; if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0) @@ -8129,7 +8129,7 @@ cmdDefine(vshControl *ctl, const vshCmd *cmd) unsigned int flags = 0; virshControl *priv = ctl->privData; - if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) + if (vshCommandOptString(ctl, cmd, "file", &from) < 0) return false; if (vshCommandOptBool(cmd, "validate")) @@ -8432,9 +8432,9 @@ cmdMetadata(vshControl *ctl, const vshCmd *cmd) if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptStringReq(ctl, cmd, "uri", &uri) < 0 || - vshCommandOptStringReq(ctl, cmd, "key", &key) < 0 || - vshCommandOptStringReq(ctl, cmd, "set", &set) < 0) + if (vshCommandOptString(ctl, cmd, "uri", &uri) < 0 || + vshCommandOptString(ctl, cmd, "key", &key) < 0 || + vshCommandOptString(ctl, cmd, "set", &set) < 0) return false; if ((set || edit) && !key) { @@ -8684,7 +8684,7 @@ cmdSendProcessSignal(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptLongLong(ctl, cmd, "pid", &pid_value) < 0) return false; - if (vshCommandOptStringReq(ctl, cmd, "signame", &signame) < 0) + if (vshCommandOptString(ctl, cmd, "signame", &signame) < 0) return false; if ((signum = getSignalNumber(signame)) < 0) { @@ -8896,7 +8896,7 @@ virshGetUpdatedMemoryXML(char **updatedMemoryXML, return -1; nodeOpt = vshCommandOptBool(cmd, "node"); - if (vshCommandOptStringReq(ctl, cmd, "alias", &alias) < 0 || + if (vshCommandOptString(ctl, cmd, "alias", &alias) < 0 || vshCommandOptUInt(ctl, cmd, "node", &node) < 0) { return -1; } @@ -9248,8 +9248,8 @@ cmdPerf(vshControl *ctl, const vshCmd *cmd) if (live) flags |= VIR_DOMAIN_AFFECT_LIVE; - if (vshCommandOptStringReq(ctl, cmd, "enable", &enable) < 0 || - vshCommandOptStringReq(ctl, cmd, "disable", &disable) < 0) + if (vshCommandOptString(ctl, cmd, "enable", &enable) < 0 || + vshCommandOptString(ctl, cmd, "disable", &disable) < 0) return false; if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) @@ -9340,7 +9340,7 @@ cmdNumatune(vshControl * ctl, const vshCmd * cmd) if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptStringReq(ctl, cmd, "nodeset", &nodeset) < 0) + if (vshCommandOptString(ctl, cmd, "nodeset", &nodeset) < 0) goto cleanup; if (nodeset && @@ -9348,7 +9348,7 @@ cmdNumatune(vshControl * ctl, const vshCmd * cmd) VIR_DOMAIN_NUMA_NODESET, nodeset) < 0) goto save_error; - if (vshCommandOptStringReq(ctl, cmd, "mode", &mode) < 0) + if (vshCommandOptString(ctl, cmd, "mode", &mode) < 0) goto cleanup; if (mode) { @@ -9502,10 +9502,10 @@ cmdDomSetLaunchSecState(vshControl * ctl, const vshCmd * cmd) if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptStringReq(ctl, cmd, "secrethdr", &sechdrfile) < 0) + if (vshCommandOptString(ctl, cmd, "secrethdr", &sechdrfile) < 0) return false; - if (vshCommandOptStringReq(ctl, cmd, "secret", &secfile) < 0) + if (vshCommandOptString(ctl, cmd, "secret", &secfile) < 0) return false; if (sechdrfile == NULL || secfile == NULL) { @@ -9609,7 +9609,7 @@ cmdDomFdAssociate(vshControl *ctl, const vshCmd *cmd) if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptStringReq(ctl, cmd, "name", &name) < 0) + if (vshCommandOptString(ctl, cmd, "name", &name) < 0) return false; if (virshFetchPassFdsList(ctl, cmd, &nfds, &fds) < 0) @@ -9941,7 +9941,7 @@ cmdQemuMonitorEvent(vshControl *ctl, const vshCmd *cmd) data.count = 0; if (vshCommandOptTimeoutToMs(ctl, cmd, &timeout) < 0) return false; - if (vshCommandOptStringReq(ctl, cmd, "event", &event) < 0) + if (vshCommandOptString(ctl, cmd, "event", &event) < 0) return false; if (vshCommandOptBool(cmd, "domain")) @@ -10350,8 +10350,8 @@ cmdDomXMLFromNative(vshControl *ctl, const vshCmd *cmd) unsigned int flags = 0; virshControl *priv = ctl->privData; - if (vshCommandOptStringReq(ctl, cmd, "format", &format) < 0 || - vshCommandOptStringReq(ctl, cmd, "config", &configFile) < 0) + if (vshCommandOptString(ctl, cmd, "format", &format) < 0 || + vshCommandOptString(ctl, cmd, "config", &configFile) < 0) return false; if (virFileReadAll(configFile, VSH_MAX_XML_FILE, &configData) < 0) @@ -10405,8 +10405,8 @@ cmdDomXMLToNative(vshControl *ctl, const vshCmd *cmd) virshControl *priv = ctl->privData; g_autoptr(virshDomain) dom = NULL; - if (vshCommandOptStringReq(ctl, cmd, "format", &format) < 0 || - vshCommandOptStringReq(ctl, cmd, "xml", &xmlFile) < 0) + if (vshCommandOptString(ctl, cmd, "format", &format) < 0 || + vshCommandOptString(ctl, cmd, "xml", &xmlFile) < 0) return false; VSH_EXCLUSIVE_OPTIONS("domain", "xml"); @@ -10500,7 +10500,7 @@ cmdDomrename(vshControl *ctl, const vshCmd *cmd) if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptStringReq(ctl, cmd, "new-name", &new_name) < 0) + if (vshCommandOptString(ctl, cmd, "new-name", &new_name) < 0) return false; if (virDomainRename(dom, new_name, 0) < 0) @@ -10881,24 +10881,24 @@ doMigrate(void *opaque) if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) goto out; - if (vshCommandOptStringReq(ctl, cmd, "desturi", &desturi) < 0) + if (vshCommandOptString(ctl, cmd, "desturi", &desturi) < 0) goto out; - if (vshCommandOptStringReq(ctl, cmd, "migrateuri", &opt) < 0) + if (vshCommandOptString(ctl, cmd, "migrateuri", &opt) < 0) goto out; if (opt && virTypedParamsAddString(¶ms, &nparams, &maxparams, VIR_MIGRATE_PARAM_URI, opt) < 0) goto save_error; - if (vshCommandOptStringReq(ctl, cmd, "graphicsuri", &opt) < 0) + if (vshCommandOptString(ctl, cmd, "graphicsuri", &opt) < 0) goto out; if (opt && virTypedParamsAddString(¶ms, &nparams, &maxparams, VIR_MIGRATE_PARAM_GRAPHICS_URI, opt) < 0) goto save_error; - if (vshCommandOptStringReq(ctl, cmd, "listen-address", &opt) < 0) + if (vshCommandOptString(ctl, cmd, "listen-address", &opt) < 0) goto out; if (opt && virTypedParamsAddString(¶ms, &nparams, &maxparams, @@ -10912,7 +10912,7 @@ doMigrate(void *opaque) VIR_MIGRATE_PARAM_DISKS_PORT, intOpt) < 0) goto save_error; - if (vshCommandOptStringReq(ctl, cmd, "disks-uri", &opt) < 0) + if (vshCommandOptString(ctl, cmd, "disks-uri", &opt) < 0) goto out; if (opt && virTypedParamsAddString(¶ms, &nparams, &maxparams, @@ -10920,14 +10920,14 @@ doMigrate(void *opaque) opt) < 0) goto save_error; - if (vshCommandOptStringReq(ctl, cmd, "dname", &opt) < 0) + if (vshCommandOptString(ctl, cmd, "dname", &opt) < 0) goto out; if (opt && virTypedParamsAddString(¶ms, &nparams, &maxparams, VIR_MIGRATE_PARAM_DEST_NAME, opt) < 0) goto save_error; - if (vshCommandOptStringReq(ctl, cmd, "migrate-disks", &opt) < 0) + if (vshCommandOptString(ctl, cmd, "migrate-disks", &opt) < 0) goto out; if (opt) { g_autofree char **val = NULL; @@ -10948,7 +10948,7 @@ doMigrate(void *opaque) } } - if (vshCommandOptStringReq(ctl, cmd, "comp-methods", &opt) < 0) + if (vshCommandOptString(ctl, cmd, "comp-methods", &opt) < 0) goto out; if (opt) { g_autofree char **val = g_strsplit(opt, ",", 0); @@ -10998,7 +10998,7 @@ doMigrate(void *opaque) goto save_error; } - if (vshCommandOptStringReq(ctl, cmd, "xml", &opt) < 0) + if (vshCommandOptString(ctl, cmd, "xml", &opt) < 0) goto out; if (opt) { g_autofree char *xml = NULL; @@ -11014,7 +11014,7 @@ doMigrate(void *opaque) } } - if (vshCommandOptStringReq(ctl, cmd, "persistent-xml", &opt) < 0) + if (vshCommandOptString(ctl, cmd, "persistent-xml", &opt) < 0) goto out; if (opt) { g_autofree char *xml = NULL; @@ -11093,7 +11093,7 @@ doMigrate(void *opaque) goto save_error; } - if (vshCommandOptStringReq(ctl, cmd, "tls-destination", &opt) < 0) + if (vshCommandOptString(ctl, cmd, "tls-destination", &opt) < 0) goto out; if (opt && virTypedParamsAddString(¶ms, &nparams, &maxparams, @@ -11250,7 +11250,7 @@ cmdMigrate(vshControl *ctl, const vshCmd *cmd) virConnectPtr dconn = NULL; const char *desturi = NULL; - if (vshCommandOptStringReq(ctl, cmd, "desturi", &desturi) < 0) + if (vshCommandOptString(ctl, cmd, "desturi", &desturi) < 0) goto cleanup; dconn = virshConnect(ctl, desturi, false); @@ -11722,7 +11722,7 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool(cmd, "include-password")) flags |= VIR_DOMAIN_XML_SECURE; - if (vshCommandOptStringReq(ctl, cmd, "type", &type) < 0) + if (vshCommandOptString(ctl, cmd, "type", &type) < 0) return false; if (virshDomainGetXMLFromDom(ctl, dom, flags, &xml, &ctxt) < 0) @@ -11885,7 +11885,7 @@ cmdDomHostname(vshControl *ctl, const vshCmd *cmd) if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptStringReq(ctl, cmd, "source", &sourcestr) < 0) + if (vshCommandOptString(ctl, cmd, "source", &sourcestr) < 0) return false; if (sourcestr) { @@ -11966,7 +11966,7 @@ cmdDetachDevice(vshControl *ctl, const vshCmd *cmd) virDomainIsActive(dom) == 1) flags |= VIR_DOMAIN_AFFECT_LIVE; - if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) + if (vshCommandOptString(ctl, cmd, "file", &from) < 0) return false; if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0) { @@ -12033,7 +12033,7 @@ cmdDetachDeviceAlias(vshControl *ctl, const vshCmd *cmd) if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptStringReq(ctl, cmd, "alias", &alias) < 0) + if (vshCommandOptString(ctl, cmd, "alias", &alias) < 0) return false; if (virDomainDetachDeviceAlias(dom, alias, flags) < 0) { @@ -12093,7 +12093,7 @@ cmdUpdateDevice(vshControl *ctl, const vshCmd *cmd) if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) + if (vshCommandOptString(ctl, cmd, "file", &from) < 0) return false; if (persistent && @@ -12255,10 +12255,10 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd) if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptStringReq(ctl, cmd, "type", &type) < 0) + if (vshCommandOptString(ctl, cmd, "type", &type) < 0) goto cleanup; - if (vshCommandOptStringReq(ctl, cmd, "mac", &mac) < 0) + if (vshCommandOptString(ctl, cmd, "mac", &mac) < 0) goto cleanup; affect_config = (config || persistent); @@ -12548,7 +12548,7 @@ cmdDetachDisk(vshControl *ctl, const vshCmd *cmd) if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptStringReq(ctl, cmd, "target", &target) < 0) + if (vshCommandOptString(ctl, cmd, "target", &target) < 0) return false; if (flags == VIR_DOMAIN_AFFECT_CONFIG) @@ -12736,7 +12736,7 @@ cmdChangeMedia(vshControl *ctl, const vshCmd *cmd) VSH_EXCLUSIVE_OPTIONS_VAR(eject, block); - if (vshCommandOptStringReq(ctl, cmd, "source", &source) < 0) + if (vshCommandOptString(ctl, cmd, "source", &source) < 0) return false; /* Docs state that update without source is eject */ @@ -12776,7 +12776,7 @@ cmdChangeMedia(vshControl *ctl, const vshCmd *cmd) if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptStringReq(ctl, cmd, "path", &path) < 0) + if (vshCommandOptString(ctl, cmd, "path", &path) < 0) return false; if (flags & VIR_DOMAIN_AFFECT_CONFIG) @@ -12842,7 +12842,7 @@ cmdDomFSTrim(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptULongLong(ctl, cmd, "minimum", &minimum) < 0) return false; - if (vshCommandOptStringReq(ctl, cmd, "mountpoint", &mountPoint) < 0) + if (vshCommandOptString(ctl, cmd, "mountpoint", &mountPoint) < 0) return false; if (virDomainFSTrim(dom, mountPoint, minimum, flags) < 0) { @@ -13164,7 +13164,7 @@ cmdGetUserSSHKeys(vshControl *ctl, const vshCmd *cmd) if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptStringReq(ctl, cmd, "user", &user) < 0) + if (vshCommandOptString(ctl, cmd, "user", &user) < 0) return false; nkeys = virDomainAuthorizedSSHKeysGet(dom, user, &keys, flags); @@ -13230,10 +13230,10 @@ cmdSetUserSSHKeys(vshControl *ctl, const vshCmd *cmd) if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptStringReq(ctl, cmd, "user", &user) < 0) + if (vshCommandOptString(ctl, cmd, "user", &user) < 0) return false; - if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) + if (vshCommandOptString(ctl, cmd, "file", &from) < 0) return false; if (vshCommandOptBool(cmd, "remove")) { @@ -13323,7 +13323,7 @@ cmdDomDirtyRateCalc(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptInt(ctl, cmd, "seconds", &seconds) < 0) return false; - if (vshCommandOptStringReq(ctl, cmd, "mode", &modestr) < 0) + if (vshCommandOptString(ctl, cmd, "mode", &modestr) < 0) return false; if (modestr) { diff --git a/tools/virsh-host.c b/tools/virsh-host.c index c338b5cd85..2fe64e415f 100644 --- a/tools/virsh-host.c +++ b/tools/virsh-host.c @@ -130,10 +130,10 @@ cmdDomCapabilities(vshControl *ctl, const vshCmd *cmd) bool wrap = vshCommandOptBool(cmd, "wrap"); virshControl *priv = ctl->privData; - if (vshCommandOptStringReq(ctl, cmd, "virttype", &virttype) < 0 || - vshCommandOptStringReq(ctl, cmd, "emulatorbin", &emulatorbin) < 0 || - vshCommandOptStringReq(ctl, cmd, "arch", &arch) < 0 || - vshCommandOptStringReq(ctl, cmd, "machine", &machine) < 0 || + if (vshCommandOptString(ctl, cmd, "virttype", &virttype) < 0 || + vshCommandOptString(ctl, cmd, "emulatorbin", &emulatorbin) < 0 || + vshCommandOptString(ctl, cmd, "arch", &arch) < 0 || + vshCommandOptString(ctl, cmd, "machine", &machine) < 0 || vshCommandOptStringQuiet(ctl, cmd, "xpath", &xpath) < 0) return false; @@ -605,7 +605,7 @@ cmdMaxvcpus(vshControl *ctl, const vshCmd *cmd) g_autoptr(xmlXPathContext) ctxt = NULL; virshControl *priv = ctl->privData; - if (vshCommandOptStringReq(ctl, cmd, "type", &type) < 0) + if (vshCommandOptString(ctl, cmd, "type", &type) < 0) return false; if ((caps = virConnectGetDomainCapabilities(priv->conn, NULL, NULL, NULL, @@ -977,7 +977,7 @@ cmdNodeSuspend(vshControl *ctl, const vshCmd *cmd) long long duration; virshControl *priv = ctl->privData; - if (vshCommandOptStringReq(ctl, cmd, "target", &target) < 0) + if (vshCommandOptString(ctl, cmd, "target", &target) < 0) return false; if (vshCommandOptLongLong(ctl, cmd, "duration", &duration) < 0) @@ -1189,7 +1189,7 @@ cmdCPUCompare(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool(cmd, "validate")) flags |= VIR_CONNECT_COMPARE_CPU_VALIDATE_XML; - if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) + if (vshCommandOptString(ctl, cmd, "file", &from) < 0) return false; if (!(cpus = vshExtractCPUDefXMLs(ctl, from))) @@ -1258,7 +1258,7 @@ cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool(cmd, "migratable")) flags |= VIR_CONNECT_BASELINE_CPU_MIGRATABLE; - if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) + if (vshCommandOptString(ctl, cmd, "file", &from) < 0) return false; if (!(list = vshExtractCPUDefXMLs(ctl, from))) @@ -1300,7 +1300,7 @@ cmdCPUModelNames(vshControl *ctl, const vshCmd *cmd) const char *arch = NULL; virshControl *priv = ctl->privData; - if (vshCommandOptStringReq(ctl, cmd, "arch", &arch) < 0) + if (vshCommandOptString(ctl, cmd, "arch", &arch) < 0) return false; nmodels = virConnectGetCPUModelNames(priv->conn, arch, &models, 0); @@ -1598,11 +1598,11 @@ cmdHypervisorCPUCompare(vshControl *ctl, if (vshCommandOptBool(cmd, "validate")) flags |= VIR_CONNECT_COMPARE_CPU_VALIDATE_XML; - if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0 || - vshCommandOptStringReq(ctl, cmd, "virttype", &virttype) < 0 || - vshCommandOptStringReq(ctl, cmd, "emulator", &emulator) < 0 || - vshCommandOptStringReq(ctl, cmd, "arch", &arch) < 0 || - vshCommandOptStringReq(ctl, cmd, "machine", &machine) < 0) + if (vshCommandOptString(ctl, cmd, "file", &from) < 0 || + vshCommandOptString(ctl, cmd, "virttype", &virttype) < 0 || + vshCommandOptString(ctl, cmd, "emulator", &emulator) < 0 || + vshCommandOptString(ctl, cmd, "arch", &arch) < 0 || + vshCommandOptString(ctl, cmd, "machine", &machine) < 0) return false; if (!(cpus = vshExtractCPUDefXMLs(ctl, from))) @@ -1718,12 +1718,12 @@ cmdHypervisorCPUBaseline(vshControl *ctl, if (vshCommandOptBool(cmd, "migratable")) flags |= VIR_CONNECT_BASELINE_CPU_MIGRATABLE; - if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0 || - vshCommandOptStringReq(ctl, cmd, "virttype", &virttype) < 0 || - vshCommandOptStringReq(ctl, cmd, "emulator", &emulator) < 0 || - vshCommandOptStringReq(ctl, cmd, "arch", &arch) < 0 || - vshCommandOptStringReq(ctl, cmd, "machine", &machine) < 0 || - vshCommandOptStringReq(ctl, cmd, "model", &model) < 0) + if (vshCommandOptString(ctl, cmd, "file", &from) < 0 || + vshCommandOptString(ctl, cmd, "virttype", &virttype) < 0 || + vshCommandOptString(ctl, cmd, "emulator", &emulator) < 0 || + vshCommandOptString(ctl, cmd, "arch", &arch) < 0 || + vshCommandOptString(ctl, cmd, "machine", &machine) < 0 || + vshCommandOptString(ctl, cmd, "model", &model) < 0) return false; VSH_ALTERNATIVE_OPTIONS_EXPR("file", from, "model", model); diff --git a/tools/virsh-interface.c b/tools/virsh-interface.c index 755c0d6455..fda6d55158 100644 --- a/tools/virsh-interface.c +++ b/tools/virsh-interface.c @@ -58,7 +58,7 @@ virshCommandOptInterfaceBy(vshControl *ctl, const vshCmd *cmd, if (!optname) optname = "interface"; - if (vshCommandOptStringReq(ctl, cmd, optname, &n) < 0) + if (vshCommandOptString(ctl, cmd, optname, &n) < 0) return NULL; vshDebug(ctl, VSH_ERR_INFO, "%s: found option <%s>: %s\n", @@ -515,7 +515,7 @@ cmdInterfaceDefine(vshControl *ctl, const vshCmd *cmd) unsigned int flags = 0; virshControl *priv = ctl->privData; - if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) + if (vshCommandOptString(ctl, cmd, "file", &from) < 0) return false; if (vshCommandOptBool(cmd, "validate")) @@ -770,7 +770,7 @@ cmdInterfaceBridge(vshControl *ctl, const vshCmd *cmd) } /* Name for new bridge device */ - if (vshCommandOptStringReq(ctl, cmd, "bridge", &br_name) < 0) + if (vshCommandOptString(ctl, cmd, "bridge", &br_name) < 0) goto cleanup; /* make sure "new" device doesn't already exist */ diff --git a/tools/virsh-network.c b/tools/virsh-network.c index e6552cbe57..24049a66f3 100644 --- a/tools/virsh-network.c +++ b/tools/virsh-network.c @@ -65,7 +65,7 @@ virshCommandOptNetworkBy(vshControl *ctl, const vshCmd *cmd, virCheckFlags(VIRSH_BYUUID | VIRSH_BYNAME, NULL); - if (vshCommandOptStringReq(ctl, cmd, optname, &n) < 0) + if (vshCommandOptString(ctl, cmd, optname, &n) < 0) return NULL; vshDebug(ctl, VSH_ERR_INFO, "%s: found option <%s>: %s\n", @@ -103,7 +103,7 @@ virshCommandOptNetworkPort(vshControl *ctl, const vshCmd *cmd, const char *n = NULL; const char *optname = "port"; - if (vshCommandOptStringReq(ctl, cmd, optname, &n) < 0) + if (vshCommandOptString(ctl, cmd, optname, &n) < 0) return NULL; vshDebug(ctl, VSH_ERR_INFO, "%s: found option <%s>: %s\n", @@ -193,7 +193,7 @@ cmdNetworkCreate(vshControl *ctl, const vshCmd *cmd) unsigned int flags = 0; virshControl *priv = ctl->privData; - if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) + if (vshCommandOptString(ctl, cmd, "file", &from) < 0) return false; if (vshCommandOptBool(cmd, "validate")) @@ -244,7 +244,7 @@ cmdNetworkDefine(vshControl *ctl, const vshCmd *cmd) unsigned int flags = 0; virshControl *priv = ctl->privData; - if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) + if (vshCommandOptString(ctl, cmd, "file", &from) < 0) return false; if (vshCommandOptBool(cmd, "validate")) @@ -570,9 +570,9 @@ cmdNetworkMetadata(vshControl *ctl, const vshCmd *cmd) if (!(net = virshCommandOptNetwork(ctl, cmd, NULL))) return false; - if (vshCommandOptStringReq(ctl, cmd, "uri", &uri) < 0 || - vshCommandOptStringReq(ctl, cmd, "key", &key) < 0 || - vshCommandOptStringReq(ctl, cmd, "set", &set) < 0) + if (vshCommandOptString(ctl, cmd, "uri", &uri) < 0 || + vshCommandOptString(ctl, cmd, "key", &key) < 0 || + vshCommandOptString(ctl, cmd, "set", &set) < 0) return false; if ((set || edit) && !key) { @@ -1281,7 +1281,7 @@ cmdNetworkUpdate(vshControl *ctl, const vshCmd *cmd) if (!(network = virshCommandOptNetwork(ctl, cmd, NULL))) return false; - if (vshCommandOptStringReq(ctl, cmd, "command", &commandStr) < 0) + if (vshCommandOptString(ctl, cmd, "command", &commandStr) < 0) goto cleanup; if (STREQ(commandStr, "add")) { @@ -1295,7 +1295,7 @@ cmdNetworkUpdate(vshControl *ctl, const vshCmd *cmd) } } - if (vshCommandOptStringReq(ctl, cmd, "section", §ionStr) < 0) + if (vshCommandOptString(ctl, cmd, "section", §ionStr) < 0) goto cleanup; section = virshNetworkSectionTypeFromString(sectionStr); @@ -1314,7 +1314,7 @@ cmdNetworkUpdate(vshControl *ctl, const vshCmd *cmd) * the desired xml. */ - if (vshCommandOptStringReq(ctl, cmd, "xml", &xml) < 0) + if (vshCommandOptString(ctl, cmd, "xml", &xml) < 0) goto cleanup; if (*xml != '<') { @@ -1631,7 +1631,7 @@ cmdNetworkEvent(vshControl *ctl, const vshCmd *cmd) return true; } - if (vshCommandOptStringReq(ctl, cmd, "event", &eventName) < 0) + if (vshCommandOptString(ctl, cmd, "event", &eventName) < 0) return false; if (!eventName) { vshError(ctl, "%s", _("either --list or --event <type> is required")); @@ -1736,7 +1736,7 @@ cmdNetworkDHCPLeases(vshControl *ctl, const vshCmd *cmd) g_autoptr(virshNetwork) network = NULL; g_autoptr(vshTable) table = NULL; - if (vshCommandOptStringReq(ctl, cmd, "mac", &mac) < 0) + if (vshCommandOptString(ctl, cmd, "mac", &mac) < 0) return false; if (!(network = virshCommandOptNetwork(ctl, cmd, &name))) @@ -1829,7 +1829,7 @@ cmdNetworkPortCreate(vshControl *ctl, const vshCmd *cmd) if (network == NULL) goto cleanup; - if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) + if (vshCommandOptString(ctl, cmd, "file", &from) < 0) goto cleanup; if (vshCommandOptBool(cmd, "validate")) diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c index b0563395f0..910eaefc9e 100644 --- a/tools/virsh-nodedev.c +++ b/tools/virsh-nodedev.c @@ -61,7 +61,7 @@ cmdNodeDeviceCreate(vshControl *ctl, const vshCmd *cmd) virshControl *priv = ctl->privData; unsigned int flags = 0; - if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) + if (vshCommandOptString(ctl, cmd, "file", &from) < 0) return false; if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0) @@ -142,7 +142,7 @@ cmdNodeDeviceDestroy(vshControl *ctl, const vshCmd *cmd) g_autoptr(virshNodeDevice) dev = NULL; const char *device_value = NULL; - if (vshCommandOptStringReq(ctl, cmd, "device", &device_value) < 0) + if (vshCommandOptString(ctl, cmd, "device", &device_value) < 0) return false; dev = vshFindNodeDevice(ctl, device_value); @@ -604,7 +604,7 @@ cmdNodeDeviceDumpXML(vshControl *ctl, const vshCmd *cmd) bool wrap = vshCommandOptBool(cmd, "wrap"); const char *xpath = NULL; - if (vshCommandOptStringReq(ctl, cmd, "device", &device_value) < 0) + if (vshCommandOptString(ctl, cmd, "device", &device_value) < 0) return false; if (vshCommandOptStringQuiet(ctl, cmd, "xpath", &xpath) < 0) @@ -659,7 +659,7 @@ cmdNodeDeviceDetach(vshControl *ctl, const vshCmd *cmd) bool ret = true; virshControl *priv = ctl->privData; - if (vshCommandOptStringReq(ctl, cmd, "device", &name) < 0) + if (vshCommandOptString(ctl, cmd, "device", &name) < 0) return false; ignore_value(vshCommandOptStringQuiet(ctl, cmd, "driver", &driverName)); @@ -716,7 +716,7 @@ cmdNodeDeviceReAttach(vshControl *ctl, const vshCmd *cmd) bool ret = true; virshControl *priv = ctl->privData; - if (vshCommandOptStringReq(ctl, cmd, "device", &name) < 0) + if (vshCommandOptString(ctl, cmd, "device", &name) < 0) return false; if (!(device = virNodeDeviceLookupByName(priv->conn, name))) { @@ -762,7 +762,7 @@ cmdNodeDeviceReset(vshControl *ctl, const vshCmd *cmd) bool ret = true; virshControl *priv = ctl->privData; - if (vshCommandOptStringReq(ctl, cmd, "device", &name) < 0) + if (vshCommandOptString(ctl, cmd, "device", &name) < 0) return false; if (!(device = virNodeDeviceLookupByName(priv->conn, name))) { @@ -936,7 +936,7 @@ cmdNodeDeviceEvent(vshControl *ctl, const vshCmd *cmd) return true; } - if (vshCommandOptStringReq(ctl, cmd, "event", &eventName) < 0) + if (vshCommandOptString(ctl, cmd, "event", &eventName) < 0) return false; if (!eventName) { vshError(ctl, "%s", _("either --list or --event <type> is required")); @@ -958,7 +958,7 @@ cmdNodeDeviceEvent(vshControl *ctl, const vshCmd *cmd) data.cb = &virshNodeDeviceEventCallbacks[event]; if (vshCommandOptTimeoutToMs(ctl, cmd, &timeout) < 0) return false; - if (vshCommandOptStringReq(ctl, cmd, "device", &device_value) < 0) + if (vshCommandOptString(ctl, cmd, "device", &device_value) < 0) return false; if (device_value) { @@ -1025,7 +1025,7 @@ cmdNodeDeviceUndefine(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED) g_autoptr(virshNodeDevice) dev = NULL; const char *device_value = NULL; - if (vshCommandOptStringReq(ctl, cmd, "device", &device_value) < 0) + if (vshCommandOptString(ctl, cmd, "device", &device_value) < 0) return false; dev = vshFindNodeDevice(ctl, device_value); @@ -1072,7 +1072,7 @@ cmdNodeDeviceDefine(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED) virshControl *priv = ctl->privData; unsigned int flags = 0; - if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) + if (vshCommandOptString(ctl, cmd, "file", &from) < 0) return false; if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0) @@ -1119,7 +1119,7 @@ cmdNodeDeviceStart(vshControl *ctl, const vshCmd *cmd) bool ret = true; virshControl *priv = ctl->privData; - if (vshCommandOptStringReq(ctl, cmd, "device", &name) < 0) + if (vshCommandOptString(ctl, cmd, "device", &name) < 0) return false; if (!(device = virNodeDeviceLookupByName(priv->conn, name))) { @@ -1168,7 +1168,7 @@ cmdNodeDeviceAutostart(vshControl *ctl, const vshCmd *cmd) const char *name = NULL; int autostart; - if (vshCommandOptStringReq(ctl, cmd, "device", &name) < 0) + if (vshCommandOptString(ctl, cmd, "device", &name) < 0) return false; dev = vshFindNodeDevice(ctl, name); @@ -1223,7 +1223,7 @@ cmdNodeDeviceInfo(vshControl *ctl, const vshCmd *cmd) int autostart; const char *parent = NULL; - if (vshCommandOptStringReq(ctl, cmd, "device", &device_value) < 0) + if (vshCommandOptString(ctl, cmd, "device", &device_value) < 0) return false; device = vshFindNodeDevice(ctl, device_value); @@ -1286,7 +1286,7 @@ cmdNodeDeviceUpdate(vshControl *ctl, const vshCmd *cmd) VSH_EXCLUSIVE_OPTIONS("current", "live"); VSH_EXCLUSIVE_OPTIONS("current", "config"); - if (vshCommandOptStringReq(ctl, cmd, "device", &device_value) < 0) + if (vshCommandOptString(ctl, cmd, "device", &device_value) < 0) return false; device = vshFindNodeDevice(ctl, device_value); @@ -1294,7 +1294,7 @@ cmdNodeDeviceUpdate(vshControl *ctl, const vshCmd *cmd) if (!device) return false; - if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) + if (vshCommandOptString(ctl, cmd, "file", &from) < 0) goto cleanup; if (virFileReadAll(from, VSH_MAX_XML_FILE, &xml) < 0) diff --git a/tools/virsh-nwfilter.c b/tools/virsh-nwfilter.c index 69473b4935..56133d6566 100644 --- a/tools/virsh-nwfilter.c +++ b/tools/virsh-nwfilter.c @@ -38,7 +38,7 @@ virshCommandOptNWFilterBy(vshControl *ctl, const vshCmd *cmd, virCheckFlags(VIRSH_BYUUID | VIRSH_BYNAME, NULL); - if (vshCommandOptStringReq(ctl, cmd, optname, &n) < 0) + if (vshCommandOptString(ctl, cmd, optname, &n) < 0) return NULL; vshDebug(ctl, VSH_ERR_INFO, "%s: found option <%s>: %s\n", @@ -94,7 +94,7 @@ cmdNWFilterDefine(vshControl *ctl, const vshCmd *cmd) unsigned int flags = 0; virshControl *priv = ctl->privData; - if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) + if (vshCommandOptString(ctl, cmd, "file", &from) < 0) return false; if (vshCommandOptBool(cmd, "validate")) @@ -447,7 +447,7 @@ virshCommandOptNWFilterBindingBy(vshControl *ctl, virCheckFlags(0, NULL); - if (vshCommandOptStringReq(ctl, cmd, optname, &n) < 0) + if (vshCommandOptString(ctl, cmd, optname, &n) < 0) return NULL; vshDebug(ctl, VSH_ERR_INFO, "%s: found option <%s>: %s\n", @@ -494,7 +494,7 @@ cmdNWFilterBindingCreate(vshControl *ctl, const vshCmd *cmd) unsigned int flags = 0; virshControl *priv = ctl->privData; - if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) + if (vshCommandOptString(ctl, cmd, "file", &from) < 0) return false; if (vshCommandOptBool(cmd, "validate")) diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index 24e7bb9bf5..f9aad8ded0 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -180,7 +180,7 @@ virshCommandOptPoolBy(vshControl *ctl, const vshCmd *cmd, const char *optname, virCheckFlags(VIRSH_BYUUID | VIRSH_BYNAME, NULL); - if (vshCommandOptStringReq(ctl, cmd, optname, &n) < 0) + if (vshCommandOptString(ctl, cmd, optname, &n) < 0) return NULL; if (cmd->skipChecks && !n) @@ -286,7 +286,7 @@ cmdPoolCreate(vshControl *ctl, const vshCmd *cmd) unsigned int flags = 0; virshControl *priv = ctl->privData; - if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) + if (vshCommandOptString(ctl, cmd, "file", &from) < 0) return false; build = vshCommandOptBool(cmd, "build"); @@ -340,31 +340,31 @@ virshBuildPoolXML(vshControl *ctl, VSH_EXCLUSIVE_OPTIONS("secret-usage", "secret-uuid"); - if (vshCommandOptStringReq(ctl, cmd, "name", &name) < 0) + if (vshCommandOptString(ctl, cmd, "name", &name) < 0) return false; - if (vshCommandOptStringReq(ctl, cmd, "type", &type) < 0) + if (vshCommandOptString(ctl, cmd, "type", &type) < 0) return false; - if (vshCommandOptStringReq(ctl, cmd, "source-host", &srcHost) < 0 || - vshCommandOptStringReq(ctl, cmd, "source-path", &srcPath) < 0 || - vshCommandOptStringReq(ctl, cmd, "source-dev", &srcDev) < 0 || - vshCommandOptStringReq(ctl, cmd, "source-name", &srcName) < 0 || - vshCommandOptStringReq(ctl, cmd, "source-format", &srcFormat) < 0 || - vshCommandOptStringReq(ctl, cmd, "target", &target) < 0 || - vshCommandOptStringReq(ctl, cmd, "auth-type", &authType) < 0 || - vshCommandOptStringReq(ctl, cmd, "auth-username", &authUsername) < 0 || - vshCommandOptStringReq(ctl, cmd, "secret-usage", &secretUsage) < 0 || - vshCommandOptStringReq(ctl, cmd, "secret-uuid", &secretUUID) < 0 || - vshCommandOptStringReq(ctl, cmd, "adapter-name", &adapterName) < 0 || - vshCommandOptStringReq(ctl, cmd, "adapter-wwnn", &adapterWwnn) < 0 || - vshCommandOptStringReq(ctl, cmd, "adapter-wwpn", &adapterWwpn) < 0 || - vshCommandOptStringReq(ctl, cmd, "adapter-parent", &adapterParent) < 0 || - vshCommandOptStringReq(ctl, cmd, "adapter-parent-wwnn", &adapterParentWwnn) < 0 || - vshCommandOptStringReq(ctl, cmd, "adapter-parent-wwpn", &adapterParentWwpn) < 0 || - vshCommandOptStringReq(ctl, cmd, "adapter-parent-fabric-wwn", &adapterParentFabricWwn) < 0 || - vshCommandOptStringReq(ctl, cmd, "source-protocol-ver", &protoVer) < 0 || - vshCommandOptStringReq(ctl, cmd, "source-initiator", &srcInitiator) < 0) { + if (vshCommandOptString(ctl, cmd, "source-host", &srcHost) < 0 || + vshCommandOptString(ctl, cmd, "source-path", &srcPath) < 0 || + vshCommandOptString(ctl, cmd, "source-dev", &srcDev) < 0 || + vshCommandOptString(ctl, cmd, "source-name", &srcName) < 0 || + vshCommandOptString(ctl, cmd, "source-format", &srcFormat) < 0 || + vshCommandOptString(ctl, cmd, "target", &target) < 0 || + vshCommandOptString(ctl, cmd, "auth-type", &authType) < 0 || + vshCommandOptString(ctl, cmd, "auth-username", &authUsername) < 0 || + vshCommandOptString(ctl, cmd, "secret-usage", &secretUsage) < 0 || + vshCommandOptString(ctl, cmd, "secret-uuid", &secretUUID) < 0 || + vshCommandOptString(ctl, cmd, "adapter-name", &adapterName) < 0 || + vshCommandOptString(ctl, cmd, "adapter-wwnn", &adapterWwnn) < 0 || + vshCommandOptString(ctl, cmd, "adapter-wwpn", &adapterWwpn) < 0 || + vshCommandOptString(ctl, cmd, "adapter-parent", &adapterParent) < 0 || + vshCommandOptString(ctl, cmd, "adapter-parent-wwnn", &adapterParentWwnn) < 0 || + vshCommandOptString(ctl, cmd, "adapter-parent-wwpn", &adapterParentWwpn) < 0 || + vshCommandOptString(ctl, cmd, "adapter-parent-fabric-wwn", &adapterParentFabricWwn) < 0 || + vshCommandOptString(ctl, cmd, "source-protocol-ver", &protoVer) < 0 || + vshCommandOptString(ctl, cmd, "source-initiator", &srcInitiator) < 0) { return false; } @@ -530,7 +530,7 @@ cmdPoolDefine(vshControl *ctl, const vshCmd *cmd) unsigned int flags = 0; virshControl *priv = ctl->privData; - if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) + if (vshCommandOptString(ctl, cmd, "file", &from) < 0) return false; if (vshCommandOptBool(cmd, "validate")) @@ -1124,7 +1124,7 @@ cmdPoolList(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED) if (vshCommandOptBool(cmd, "name")) name = true; - if (vshCommandOptStringReq(ctl, cmd, "type", &type) < 0) + if (vshCommandOptString(ctl, cmd, "type", &type) < 0) return false; VSH_EXCLUSIVE_OPTIONS("details", "uuid"); @@ -1407,16 +1407,16 @@ cmdPoolDiscoverSourcesAs(vshControl * ctl, const vshCmd * cmd G_GNUC_UNUSED) const char *initiator = NULL; virshControl *priv = ctl->privData; - if (vshCommandOptStringReq(ctl, cmd, "type", &type) < 0 || - vshCommandOptStringReq(ctl, cmd, "host", &host) < 0 || - vshCommandOptStringReq(ctl, cmd, "initiator", &initiator) < 0) + if (vshCommandOptString(ctl, cmd, "type", &type) < 0 || + vshCommandOptString(ctl, cmd, "host", &host) < 0 || + vshCommandOptString(ctl, cmd, "initiator", &initiator) < 0) return false; if (host) { const char *port = NULL; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; - if (vshCommandOptStringReq(ctl, cmd, "port", &port) < 0) { + if (vshCommandOptString(ctl, cmd, "port", &port) < 0) { vshError(ctl, "%s", _("missing argument")); return false; } @@ -1480,10 +1480,10 @@ cmdPoolDiscoverSources(vshControl * ctl, const vshCmd * cmd G_GNUC_UNUSED) char *srcSpec = NULL, *srcList; virshControl *priv = ctl->privData; - if (vshCommandOptStringReq(ctl, cmd, "type", &type) < 0) + if (vshCommandOptString(ctl, cmd, "type", &type) < 0) return false; - if (vshCommandOptStringReq(ctl, cmd, "srcSpec", &srcSpecFile) < 0) + if (vshCommandOptString(ctl, cmd, "srcSpec", &srcSpecFile) < 0) return false; if (srcSpecFile && virFileReadAll(srcSpecFile, VSH_MAX_XML_FILE, @@ -1952,7 +1952,7 @@ cmdPoolEvent(vshControl *ctl, const vshCmd *cmd) return true; } - if (vshCommandOptStringReq(ctl, cmd, "event", &eventName) < 0) + if (vshCommandOptString(ctl, cmd, "event", &eventName) < 0) return false; if (!eventName) { vshError(ctl, "%s", _("either --list or --event <type> is required")); diff --git a/tools/virsh-secret.c b/tools/virsh-secret.c index 5653451862..d9435e4357 100644 --- a/tools/virsh-secret.c +++ b/tools/virsh-secret.c @@ -41,7 +41,7 @@ virshCommandOptSecret(vshControl *ctl, const vshCmd *cmd, const char **name) const char *optname = "secret"; virshControl *priv = ctl->privData; - if (vshCommandOptStringReq(ctl, cmd, optname, &n) < 0) + if (vshCommandOptString(ctl, cmd, optname, &n) < 0) return NULL; vshDebug(ctl, VSH_ERR_DEBUG, @@ -86,7 +86,7 @@ cmdSecretDefine(vshControl *ctl, const vshCmd *cmd) unsigned int flags = 0; virshControl *priv = ctl->privData; - if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) + if (vshCommandOptString(ctl, cmd, "file", &from) < 0) return false; if (vshCommandOptBool(cmd, "validate")) @@ -227,10 +227,10 @@ cmdSecretSetValue(vshControl *ctl, const vshCmd *cmd) if (!(secret = virshCommandOptSecret(ctl, cmd, NULL))) return false; - if (vshCommandOptStringReq(ctl, cmd, "base64", &base64) < 0) + if (vshCommandOptString(ctl, cmd, "base64", &base64) < 0) return false; - if (vshCommandOptStringReq(ctl, cmd, "file", &filename) < 0) + if (vshCommandOptString(ctl, cmd, "file", &filename) < 0) return false; if (base64) { @@ -763,7 +763,7 @@ cmdSecretEvent(vshControl *ctl, const vshCmd *cmd) return true; } - if (vshCommandOptStringReq(ctl, cmd, "event", &eventName) < 0) + if (vshCommandOptString(ctl, cmd, "event", &eventName) < 0) return false; if (!eventName) { vshError(ctl, "%s", _("either --list or --event <type> is required")); diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c index 80448ca08d..415a390786 100644 --- a/tools/virsh-snapshot.c +++ b/tools/virsh-snapshot.c @@ -182,7 +182,7 @@ cmdSnapshotCreate(vshControl *ctl, const vshCmd *cmd) if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptStringReq(ctl, cmd, "xmlfile", &from) < 0) + if (vshCommandOptString(ctl, cmd, "xmlfile", &from) < 0) return false; if (!from) { buffer = g_strdup("<domainsnapshot/>"); @@ -401,8 +401,8 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd) if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptStringReq(ctl, cmd, "name", &name) < 0 || - vshCommandOptStringReq(ctl, cmd, "description", &desc) < 0) + if (vshCommandOptString(ctl, cmd, "name", &name) < 0 || + vshCommandOptString(ctl, cmd, "description", &desc) < 0) return false; virBufferAddLit(&buf, "<domainsnapshot>\n"); @@ -410,7 +410,7 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd) virBufferEscapeString(&buf, "<name>%s</name>\n", name); virBufferEscapeString(&buf, "<description>%s</description>\n", desc); - if (vshCommandOptStringReq(ctl, cmd, "memspec", &memspec) < 0) + if (vshCommandOptString(ctl, cmd, "memspec", &memspec) < 0) return false; if (memspec && virshParseSnapshotMemspec(ctl, &buf, memspec) < 0) @@ -451,7 +451,7 @@ virshLookupSnapshot(vshControl *ctl, const vshCmd *cmd, bool current = vshCommandOptBool(cmd, "current"); const char *snapname = NULL; - if (vshCommandOptStringReq(ctl, cmd, arg, &snapname) < 0) + if (vshCommandOptString(ctl, cmd, arg, &snapname) < 0) return -1; if (exclusive && current && snapname) { @@ -629,7 +629,7 @@ cmdSnapshotCurrent(vshControl *ctl, const vshCmd *cmd) if (!(dom = virshCommandOptDomain(ctl, cmd, &domname))) return false; - if (vshCommandOptStringReq(ctl, cmd, "snapshotname", &snapshotname) < 0) + if (vshCommandOptString(ctl, cmd, "snapshotname", &snapshotname) < 0) goto cleanup; if (snapshotname) { @@ -1616,7 +1616,7 @@ cmdSnapshotDumpXML(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool(cmd, "security-info")) flags |= VIR_DOMAIN_XML_SECURE; - if (vshCommandOptStringReq(ctl, cmd, "snapshotname", &name) < 0) + if (vshCommandOptString(ctl, cmd, "snapshotname", &name) < 0) return false; if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) diff --git a/tools/virsh-util.c b/tools/virsh-util.c index a6026eed53..ab350f0326 100644 --- a/tools/virsh-util.c +++ b/tools/virsh-util.c @@ -87,7 +87,7 @@ virshCommandOptDomainBy(vshControl *ctl, const char *n = NULL; const char *optname = "domain"; - if (vshCommandOptStringReq(ctl, cmd, optname, &n) < 0) + if (vshCommandOptString(ctl, cmd, optname, &n) < 0) return NULL; vshDebug(ctl, VSH_ERR_INFO, "%s: found option <%s>: %s\n", diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index 67a6f2eda0..8805b8c06b 100644 --- a/tools/virsh-volume.c +++ b/tools/virsh-volume.c @@ -92,11 +92,11 @@ virshCommandOptVolBy(vshControl *ctl, const vshCmd *cmd, virCheckFlags(VIRSH_BYUUID | VIRSH_BYNAME, NULL); - if (vshCommandOptStringReq(ctl, cmd, optname, &n) < 0) + if (vshCommandOptString(ctl, cmd, optname, &n) < 0) return NULL; if (pooloptname != NULL && - vshCommandOptStringReq(ctl, cmd, pooloptname, &p) < 0) + vshCommandOptString(ctl, cmd, pooloptname, &p) < 0) return NULL; if (p) { @@ -249,10 +249,10 @@ cmdVolCreateAs(vshControl *ctl, const vshCmd *cmd) if (!(pool = virshCommandOptPool(ctl, cmd, "pool", NULL))) return false; - if (vshCommandOptStringReq(ctl, cmd, "name", &name) < 0) + if (vshCommandOptString(ctl, cmd, "name", &name) < 0) return false; - if (vshCommandOptStringReq(ctl, cmd, "capacity", &capacityStr) < 0) + if (vshCommandOptString(ctl, cmd, "capacity", &capacityStr) < 0) return false; if (virshVolSize(capacityStr, &capacity) < 0) { @@ -266,10 +266,9 @@ cmdVolCreateAs(vshControl *ctl, const vshCmd *cmd) return false; } - if (vshCommandOptStringReq(ctl, cmd, "format", &format) < 0 || - vshCommandOptStringReq(ctl, cmd, "backing-vol", &snapshotStrVol) < 0 || - vshCommandOptStringReq(ctl, cmd, "backing-vol-format", - &snapshotStrFormat) < 0) + if (vshCommandOptString(ctl, cmd, "format", &format) < 0 || + vshCommandOptString(ctl, cmd, "backing-vol", &snapshotStrVol) < 0 || + vshCommandOptString(ctl, cmd, "backing-vol-format", &snapshotStrFormat) < 0) return false; virBufferAddLit(&buf, "<volume>\n"); @@ -404,7 +403,7 @@ cmdVolCreate(vshControl *ctl, const vshCmd *cmd) if (!(pool = virshCommandOptPool(ctl, cmd, "pool", NULL))) return false; - if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) + if (vshCommandOptString(ctl, cmd, "file", &from) < 0) return false; if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0) { @@ -477,7 +476,7 @@ cmdVolCreateFrom(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool(cmd, "validate")) flags |= VIR_STORAGE_VOL_CREATE_VALIDATE; - if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) + if (vshCommandOptString(ctl, cmd, "file", &from) < 0) return false; if (!(inputvol = virshCommandOptVol(ctl, cmd, "vol", "inputpool", NULL))) @@ -577,7 +576,7 @@ cmdVolClone(vshControl *ctl, const vshCmd *cmd) return false; } - if (vshCommandOptStringReq(ctl, cmd, "newname", &name) < 0) + if (vshCommandOptString(ctl, cmd, "newname", &name) < 0) return false; if (!(origxml = virStorageVolGetXMLDesc(origvol, 0))) @@ -656,7 +655,7 @@ cmdVolUpload(vshControl *ctl, const vshCmd *cmd) if (!(vol = virshCommandOptVol(ctl, cmd, "vol", "pool", &name))) return false; - if (vshCommandOptStringReq(ctl, cmd, "file", &file) < 0) + if (vshCommandOptString(ctl, cmd, "file", &file) < 0) return false; if ((fd = open(file, O_RDONLY)) < 0) { @@ -768,7 +767,7 @@ cmdVolDownload(vshControl *ctl, const vshCmd *cmd) if (!(vol = virshCommandOptVol(ctl, cmd, "vol", "pool", &name))) return false; - if (vshCommandOptStringReq(ctl, cmd, "file", &file) < 0) + if (vshCommandOptString(ctl, cmd, "file", &file) < 0) goto cleanup; if (vshCommandOptBool(cmd, "sparse")) @@ -908,7 +907,7 @@ cmdVolWipe(vshControl *ctl, const vshCmd *cmd) if (!(vol = virshCommandOptVol(ctl, cmd, "vol", "pool", &name))) return false; - if (vshCommandOptStringReq(ctl, cmd, "algorithm", &algorithm_str) < 0) + if (vshCommandOptString(ctl, cmd, "algorithm", &algorithm_str) < 0) return false; if (algorithm_str && @@ -1078,7 +1077,7 @@ cmdVolResize(vshControl *ctl, const vshCmd *cmd) if (!(vol = virshCommandOptVol(ctl, cmd, "vol", "pool", NULL))) return false; - if (vshCommandOptStringReq(ctl, cmd, "capacity", &capacityStr) < 0) + if (vshCommandOptString(ctl, cmd, "capacity", &capacityStr) < 0) return false; virSkipSpaces(&capacityStr); if (*capacityStr == '-') { diff --git a/tools/virsh.c b/tools/virsh.c index 0a586fd639..58ebb493fc 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -269,7 +269,7 @@ cmdConnect(vshControl *ctl, const vshCmd *cmd) bool ro = vshCommandOptBool(cmd, "readonly"); const char *name = NULL; - if (vshCommandOptStringReq(ctl, cmd, "name", &name) < 0) + if (vshCommandOptString(ctl, cmd, "name", &name) < 0) return false; if (virshReconnect(ctl, name, ro, true) < 0) diff --git a/tools/virt-admin.c b/tools/virt-admin.c index 9a10a4eb45..a996923094 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -273,7 +273,7 @@ cmdConnect(vshControl *ctl, const vshCmd *cmd) vshAdmControl *priv = ctl->privData; bool connected = priv->conn; - if (vshCommandOptStringReq(ctl, cmd, "name", &name) < 0) + if (vshCommandOptString(ctl, cmd, "name", &name) < 0) return false; if (name) { @@ -379,7 +379,7 @@ cmdSrvThreadpoolInfo(vshControl *ctl, const vshCmd *cmd) virAdmServerPtr srv = NULL; vshAdmControl *priv = ctl->privData; - if (vshCommandOptStringReq(ctl, cmd, "server", &srvname) < 0) + if (vshCommandOptString(ctl, cmd, "server", &srvname) < 0) return false; if (!(srv = virAdmConnectLookupServer(priv->conn, srvname, 0))) @@ -454,7 +454,7 @@ cmdSrvThreadpoolSet(vshControl *ctl, const vshCmd *cmd) virAdmServerPtr srv = NULL; vshAdmControl *priv = ctl->privData; - if (vshCommandOptStringReq(ctl, cmd, "server", &srvname) < 0) + if (vshCommandOptString(ctl, cmd, "server", &srvname) < 0) return false; #define PARSE_CMD_TYPED_PARAM(NAME, FIELD) \ @@ -545,7 +545,7 @@ cmdSrvClientsList(vshControl *ctl, const vshCmd *cmd) vshAdmControl *priv = ctl->privData; g_autoptr(vshTable) table = NULL; - if (vshCommandOptStringReq(ctl, cmd, "server", &srvname) < 0) + if (vshCommandOptString(ctl, cmd, "server", &srvname) < 0) return false; if (!(srv = virAdmConnectLookupServer(priv->conn, srvname, 0))) @@ -638,7 +638,7 @@ cmdClientInfo(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptULongLong(ctl, cmd, "client", &id) < 0) return false; - if (vshCommandOptStringReq(ctl, cmd, "server", &srvname) < 0) + if (vshCommandOptString(ctl, cmd, "server", &srvname) < 0) return false; if (!(srv = virAdmConnectLookupServer(priv->conn, srvname, 0)) || @@ -714,7 +714,7 @@ cmdClientDisconnect(vshControl *ctl, const vshCmd *cmd) virAdmClientPtr client = NULL; vshAdmControl *priv = ctl->privData; - if (vshCommandOptStringReq(ctl, cmd, "server", &srvname) < 0) + if (vshCommandOptString(ctl, cmd, "server", &srvname) < 0) return false; if (vshCommandOptULongLongWrap(ctl, cmd, "client", &id) < 0) @@ -772,7 +772,7 @@ cmdSrvClientsInfo(vshControl *ctl, const vshCmd *cmd) virAdmServerPtr srv = NULL; vshAdmControl *priv = ctl->privData; - if (vshCommandOptStringReq(ctl, cmd, "server", &srvname) < 0) + if (vshCommandOptString(ctl, cmd, "server", &srvname) < 0) return false; if (!(srv = virAdmConnectLookupServer(priv->conn, srvname, 0))) @@ -841,7 +841,7 @@ cmdSrvClientsSet(vshControl *ctl, const vshCmd *cmd) virTypedParameterPtr params = NULL; vshAdmControl *priv = ctl->privData; - if (vshCommandOptStringReq(ctl, cmd, "server", &srvname) < 0) + if (vshCommandOptString(ctl, cmd, "server", &srvname) < 0) return false; #define PARSE_CMD_TYPED_PARAM(NAME, FIELD) \ @@ -925,7 +925,7 @@ cmdSrvUpdateTlsFiles(vshControl *ctl, const vshCmd *cmd) virAdmServerPtr srv = NULL; vshAdmControl *priv = ctl->privData; - if (vshCommandOptStringReq(ctl, cmd, "server", &srvname) < 0) + if (vshCommandOptString(ctl, cmd, "server", &srvname) < 0) return false; if (!(srv = virAdmConnectLookupServer(priv->conn, srvname, 0))) @@ -973,7 +973,7 @@ cmdDaemonLogFilters(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool(cmd, "filters")) { const char *filters = NULL; - if ((vshCommandOptStringReq(ctl, cmd, "filters", &filters) < 0 || + if ((vshCommandOptString(ctl, cmd, "filters", &filters) < 0 || virAdmConnectSetLoggingFilters(priv->conn, filters, 0) < 0)) { vshError(ctl, _("Unable to change daemon logging settings")); return false; @@ -1056,7 +1056,7 @@ cmdDaemonLogOutputs(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool(cmd, "outputs")) { const char *outputs = NULL; - if ((vshCommandOptStringReq(ctl, cmd, "outputs", &outputs) < 0 || + if ((vshCommandOptString(ctl, cmd, "outputs", &outputs) < 0 || virAdmConnectSetLoggingOutputs(priv->conn, outputs, 0) < 0)) { vshError(ctl, _("Unable to change daemon logging settings")); return false; diff --git a/tools/vsh.c b/tools/vsh.c index 2805574ec6..d6c05c46e6 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -1036,7 +1036,7 @@ vshCommandOptStringQuiet(vshControl *ctl G_GNUC_UNUSED, const vshCmd *cmd, } /** - * vshCommandOptStringReq: + * vshCommandOptString: * @ctl virtshell control structure * @cmd command structure * @name option name @@ -1049,10 +1049,10 @@ vshCommandOptStringQuiet(vshControl *ctl G_GNUC_UNUSED, const vshCmd *cmd, * returned and error message printed. */ int -vshCommandOptStringReq(vshControl *ctl, - const vshCmd *cmd, - const char *name, - const char **value) +vshCommandOptString(vshControl *ctl, + const vshCmd *cmd, + const char *name, + const char **value) { vshCmdOpt *arg; int ret; diff --git a/tools/vsh.h b/tools/vsh.h index eeba1d4b3c..a51140eeee 100644 --- a/tools/vsh.h +++ b/tools/vsh.h @@ -265,7 +265,7 @@ int vshCommandOptULWrap(vshControl *ctl, const vshCmd *cmd, int vshCommandOptStringQuiet(vshControl *ctl, const vshCmd *cmd, const char *name, const char **value) ATTRIBUTE_NONNULL(4) G_GNUC_WARN_UNUSED_RESULT; -int vshCommandOptStringReq(vshControl *ctl, const vshCmd *cmd, +int vshCommandOptString(vshControl *ctl, const vshCmd *cmd, const char *name, const char **value) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) G_GNUC_WARN_UNUSED_RESULT; -- 2.44.0

On 4/19/24 15:05, Peter Krempa wrote:
Shorten the function name as there isn't any vshCommandOptString.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/virsh-backup.c | 4 +- tools/virsh-checkpoint.c | 8 +- tools/virsh-completer-domain.c | 2 +- tools/virsh-completer-host.c | 8 +- tools/virsh-domain-event.c | 2 +- tools/virsh-domain-monitor.c | 10 +- tools/virsh-domain.c | 248 ++++++++++++++++----------------- tools/virsh-host.c | 40 +++--- tools/virsh-interface.c | 6 +- tools/virsh-network.c | 26 ++-- tools/virsh-nodedev.c | 30 ++-- tools/virsh-nwfilter.c | 8 +- tools/virsh-pool.c | 64 ++++----- tools/virsh-secret.c | 10 +- tools/virsh-snapshot.c | 14 +- tools/virsh-util.c | 2 +- tools/virsh-volume.c | 29 ++-- tools/virsh.c | 2 +- tools/virt-admin.c | 22 +-- tools/vsh.c | 10 +- tools/vsh.h | 2 +- 21 files changed, 273 insertions(+), 274 deletions(-)
diff --git a/tools/vsh.h b/tools/vsh.h index eeba1d4b3c..a51140eeee 100644 --- a/tools/vsh.h +++ b/tools/vsh.h @@ -265,7 +265,7 @@ int vshCommandOptULWrap(vshControl *ctl, const vshCmd *cmd, int vshCommandOptStringQuiet(vshControl *ctl, const vshCmd *cmd, const char *name, const char **value) ATTRIBUTE_NONNULL(4) G_GNUC_WARN_UNUSED_RESULT; -int vshCommandOptStringReq(vshControl *ctl, const vshCmd *cmd, +int vshCommandOptString(vshControl *ctl, const vshCmd *cmd, const char *name, const char **value)
Indentation.
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) G_GNUC_WARN_UNUSED_RESULT;
Michal

While the 'complete' command is meant to be hidden and used only for the completion script, there's nothing preventing it being used in all virsh modes. This poses a problem as the command tries to close 'stdin' to avoid the possibility that an auth callback would want to read the password. In interactive mode this immediately terminates virsh and in non-interactive mode it attempts to close it multiple times if you use virsh in batch mode. Fix the issues by using virOnce() to close it exactly once and do so only in non-interactive mode. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/vsh.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/tools/vsh.c b/tools/vsh.c index d6c05c46e6..8719875413 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -3401,11 +3401,25 @@ const vshCmdInfo info_complete = { #ifdef WITH_READLINE + +static virOnceControl vshCmdCompleteCloseStdinOnce = VIR_ONCE_CONTROL_INITIALIZER; + +static void +vshCmdCompleteCloseStdin(void) +{ + /* In non-interactive mode which is how the 'complete' command is intended + * to be used we need to ensure that any authentication callback will not + * attempt to read any input which would break the completion */ + int stdin_fileno = STDIN_FILENO; + + VIR_FORCE_CLOSE(stdin_fileno); +} + + bool cmdComplete(vshControl *ctl, const vshCmd *cmd) { const vshClientHooks *hooks = ctl->hooks; - int stdin_fileno = STDIN_FILENO; const char *arg = ""; const vshCmdOpt *opt = NULL; g_auto(GStrv) matches = NULL; @@ -3418,7 +3432,10 @@ cmdComplete(vshControl *ctl, const vshCmd *cmd) /* This command is flagged VSH_CMD_FLAG_NOCONNECT because we * need to prevent auth hooks reading any input. Therefore, we * have to close stdin and then connect ourselves. */ - VIR_FORCE_CLOSE(stdin_fileno); + if (!ctl->imode) { + if (virOnce(&vshCmdCompleteCloseStdinOnce, vshCmdCompleteCloseStdin) < 0) + return false; + } if (!(hooks && hooks->connHandler && hooks->connHandler(ctl))) return false; -- 2.44.0

The argument will be used for testing the command/option completer function. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/vsh.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tools/vsh.c b/tools/vsh.c index 8719875413..71c0778660 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -3246,6 +3246,10 @@ const vshCmdOptDef opts_echo[] = { .type = VSH_OT_ALIAS, .help = "string=hello" }, + {.name = "prefix", + .type = VSH_OT_STRING, + .help = N_("prefix the message") + }, {.name = "string", .type = VSH_OT_ARGV, .positional = true, @@ -3269,6 +3273,7 @@ cmdEcho(vshControl *ctl, const vshCmd *cmd) bool xml = vshCommandOptBool(cmd, "xml"); bool err = vshCommandOptBool(cmd, "err"); bool split = vshCommandOptBool(cmd, "split"); + const char *prefix; const vshCmdOpt *opt = NULL; g_autofree char *arg = NULL; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; @@ -3277,6 +3282,11 @@ cmdEcho(vshControl *ctl, const vshCmd *cmd) VSH_EXCLUSIVE_OPTIONS_VAR(shell, split); VSH_EXCLUSIVE_OPTIONS_VAR(xml, split); + ignore_value(vshCommandOptString(ctl, cmd, "prefix", &prefix)); + + if (prefix) + virBufferAsprintf(&buf, "%s ", prefix); + while ((opt = vshCommandOptArgv(ctl, cmd, opt))) { const char *curr = opt->data; -- 2.44.0

Add both single invocations as well as a script containing the same commands. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/virshtest.c | 37 ++++++++++ .../completion-arg-full-argv-next.out | 2 + .../completion-arg-full-argv.out | 2 + .../completion-arg-full-bool-next.out | 7 ++ .../completion-arg-full-bool.out | 2 + .../completion-arg-full-string-next.out | 7 ++ .../completion-arg-full-string.out | 2 + .../virshtestdata/completion-arg-partial.out | 4 ++ .../completion-arg-positional-empty.out | 3 + ...completion-arg-positional-partial-next.out | 2 + .../completion-arg-positional-partial.out | 2 + tests/virshtestdata/completion-args.out | 7 ++ .../completion-argv-multiple-next.out | 25 +++++++ ...mpletion-argv-multiple-positional-next.out | 25 +++++++ .../completion-argv-multiple-positional.out | 2 + .../completion-argv-multiple.out | 2 + .../completion-command-complete.out | 2 + tests/virshtestdata/completion-command.out | 2 + tests/virshtestdata/completion.in | 16 +++++ tests/virshtestdata/completion.out | 71 +++++++++++++++++++ 20 files changed, 222 insertions(+) create mode 100644 tests/virshtestdata/completion-arg-full-argv-next.out create mode 100644 tests/virshtestdata/completion-arg-full-argv.out create mode 100644 tests/virshtestdata/completion-arg-full-bool-next.out create mode 100644 tests/virshtestdata/completion-arg-full-bool.out create mode 100644 tests/virshtestdata/completion-arg-full-string-next.out create mode 100644 tests/virshtestdata/completion-arg-full-string.out create mode 100644 tests/virshtestdata/completion-arg-partial.out create mode 100644 tests/virshtestdata/completion-arg-positional-empty.out create mode 100644 tests/virshtestdata/completion-arg-positional-partial-next.out create mode 100644 tests/virshtestdata/completion-arg-positional-partial.out create mode 100644 tests/virshtestdata/completion-args.out create mode 100644 tests/virshtestdata/completion-argv-multiple-next.out create mode 100644 tests/virshtestdata/completion-argv-multiple-positional-next.out create mode 100644 tests/virshtestdata/completion-argv-multiple-positional.out create mode 100644 tests/virshtestdata/completion-argv-multiple.out create mode 100644 tests/virshtestdata/completion-command-complete.out create mode 100644 tests/virshtestdata/completion-command.out create mode 100644 tests/virshtestdata/completion.in create mode 100644 tests/virshtestdata/completion.out diff --git a/tests/virshtest.c b/tests/virshtest.c index 14a96f2d35..869ecbb358 100644 --- a/tests/virshtest.c +++ b/tests/virshtest.c @@ -342,6 +342,43 @@ mymain(void) "checkpoint-create test --redefine checkpoint-c2.xml ;" "checkpoint-info test c2"); + DO_TEST_FULL("completion-command", NULL, VIRSH_CUSTOM, + "complete", "--", "ech"); + DO_TEST_FULL("completion-command-complete", NULL, VIRSH_CUSTOM, + "complete", "--", "echo"); + DO_TEST_FULL("completion-args", NULL, VIRSH_CUSTOM, + "complete", "--", "echo", ""); + DO_TEST_FULL("completion-arg-partial", NULL, VIRSH_CUSTOM, + "complete", "--", "echo", "--xm", "--s"); + DO_TEST_FULL("completion-arg-full-bool", NULL, VIRSH_CUSTOM, + "complete", "--", "echo", "--nonexistant-arg", "--xml"); + DO_TEST_FULL("completion-arg-full-bool-next", NULL, VIRSH_CUSTOM, + "complete", "--", "echo", "--nonexistant-arg", "--xml", ""); + DO_TEST_FULL("completion-arg-full-string", NULL, VIRSH_CUSTOM, + "complete", "--", "echo", "--nonexistant-arg", "--prefix"); + DO_TEST_FULL("completion-arg-full-string-next", NULL, VIRSH_CUSTOM, + "complete", "--", "echo", "--nonexistant-arg", "--prefix", ""); + DO_TEST_FULL("completion-arg-full-argv", NULL, VIRSH_CUSTOM, + "complete", "--", "domstats", "--domain"); + DO_TEST_FULL("completion-arg-full-argv-next", NULL, VIRSH_DEFAULT, + "complete", "--", "domstats", "--domain", ""); + DO_TEST_FULL("completion-argv-multiple", NULL, VIRSH_CUSTOM, + "complete", "--", "domstats", "--domain", "fc", "--domain", "fv"); + DO_TEST_FULL("completion-argv-multiple-next", NULL, VIRSH_CUSTOM, + "complete", "--", "domstats", "--domain", "fv", "--domain", "fc", ""); + DO_TEST_FULL("completion-argv-multiple-positional", NULL, VIRSH_CUSTOM, + "complete", "--", "domstats", "--domain", "fc", "fv"); + DO_TEST_FULL("completion-argv-multiple-positional-next", NULL, VIRSH_CUSTOM, + "complete", "--", "domstats", "--domain", "fc", "fv", ""); + DO_TEST_FULL("completion-arg-positional-empty", NULL, VIRSH_CUSTOM, + "complete", "--", "domstate", ""); + DO_TEST_FULL("completion-arg-positional-partial", NULL, VIRSH_CUSTOM, + "complete", "--", "domstate", "fv"); + DO_TEST_FULL("completion-arg-positional-partial-next", NULL, VIRSH_CUSTOM, + "complete", "--", "domstate", "fv", ""); + + DO_TEST_SCRIPT("completion", NULL, VIRSH_DEFAULT); + if (virTestRun("read-big-pipe", testVirshPipe, NULL) < 0) ret = -1; diff --git a/tests/virshtestdata/completion-arg-full-argv-next.out b/tests/virshtestdata/completion-arg-full-argv-next.out new file mode 100644 index 0000000000..76e579ae4c --- /dev/null +++ b/tests/virshtestdata/completion-arg-full-argv-next.out @@ -0,0 +1,2 @@ +test + diff --git a/tests/virshtestdata/completion-arg-full-argv.out b/tests/virshtestdata/completion-arg-full-argv.out new file mode 100644 index 0000000000..2d795410c2 --- /dev/null +++ b/tests/virshtestdata/completion-arg-full-argv.out @@ -0,0 +1,2 @@ +--domain + diff --git a/tests/virshtestdata/completion-arg-full-bool-next.out b/tests/virshtestdata/completion-arg-full-bool-next.out new file mode 100644 index 0000000000..3fc6e035d0 --- /dev/null +++ b/tests/virshtestdata/completion-arg-full-bool-next.out @@ -0,0 +1,7 @@ +--shell +--xml +--split +--err +--prefix +--string + diff --git a/tests/virshtestdata/completion-arg-full-bool.out b/tests/virshtestdata/completion-arg-full-bool.out new file mode 100644 index 0000000000..511582e4c9 --- /dev/null +++ b/tests/virshtestdata/completion-arg-full-bool.out @@ -0,0 +1,2 @@ +--xml + diff --git a/tests/virshtestdata/completion-arg-full-string-next.out b/tests/virshtestdata/completion-arg-full-string-next.out new file mode 100644 index 0000000000..3fc6e035d0 --- /dev/null +++ b/tests/virshtestdata/completion-arg-full-string-next.out @@ -0,0 +1,7 @@ +--shell +--xml +--split +--err +--prefix +--string + diff --git a/tests/virshtestdata/completion-arg-full-string.out b/tests/virshtestdata/completion-arg-full-string.out new file mode 100644 index 0000000000..b29473399a --- /dev/null +++ b/tests/virshtestdata/completion-arg-full-string.out @@ -0,0 +1,2 @@ +--prefix + diff --git a/tests/virshtestdata/completion-arg-partial.out b/tests/virshtestdata/completion-arg-partial.out new file mode 100644 index 0000000000..cc2820d9a8 --- /dev/null +++ b/tests/virshtestdata/completion-arg-partial.out @@ -0,0 +1,4 @@ +--shell +--split +--string + diff --git a/tests/virshtestdata/completion-arg-positional-empty.out b/tests/virshtestdata/completion-arg-positional-empty.out new file mode 100644 index 0000000000..cf2bdf1586 --- /dev/null +++ b/tests/virshtestdata/completion-arg-positional-empty.out @@ -0,0 +1,3 @@ +--domain +--reason + diff --git a/tests/virshtestdata/completion-arg-positional-partial-next.out b/tests/virshtestdata/completion-arg-positional-partial-next.out new file mode 100644 index 0000000000..d9e9cd0a05 --- /dev/null +++ b/tests/virshtestdata/completion-arg-positional-partial-next.out @@ -0,0 +1,2 @@ +--reason + diff --git a/tests/virshtestdata/completion-arg-positional-partial.out b/tests/virshtestdata/completion-arg-positional-partial.out new file mode 100644 index 0000000000..4289a724d9 --- /dev/null +++ b/tests/virshtestdata/completion-arg-positional-partial.out @@ -0,0 +1,2 @@ +fv0 + diff --git a/tests/virshtestdata/completion-args.out b/tests/virshtestdata/completion-args.out new file mode 100644 index 0000000000..3fc6e035d0 --- /dev/null +++ b/tests/virshtestdata/completion-args.out @@ -0,0 +1,7 @@ +--shell +--xml +--split +--err +--prefix +--string + diff --git a/tests/virshtestdata/completion-argv-multiple-next.out b/tests/virshtestdata/completion-argv-multiple-next.out new file mode 100644 index 0000000000..b29a060eff --- /dev/null +++ b/tests/virshtestdata/completion-argv-multiple-next.out @@ -0,0 +1,25 @@ +--state +--cpu-total +--balloon +--vcpu +--interface +--block +--perf +--iothread +--memory +--dirtyrate +--vm +--list-active +--list-inactive +--list-persistent +--list-transient +--list-running +--list-paused +--list-shutoff +--list-other +--raw +--enforce +--backing +--nowait +--domain + diff --git a/tests/virshtestdata/completion-argv-multiple-positional-next.out b/tests/virshtestdata/completion-argv-multiple-positional-next.out new file mode 100644 index 0000000000..b29a060eff --- /dev/null +++ b/tests/virshtestdata/completion-argv-multiple-positional-next.out @@ -0,0 +1,25 @@ +--state +--cpu-total +--balloon +--vcpu +--interface +--block +--perf +--iothread +--memory +--dirtyrate +--vm +--list-active +--list-inactive +--list-persistent +--list-transient +--list-running +--list-paused +--list-shutoff +--list-other +--raw +--enforce +--backing +--nowait +--domain + diff --git a/tests/virshtestdata/completion-argv-multiple-positional.out b/tests/virshtestdata/completion-argv-multiple-positional.out new file mode 100644 index 0000000000..4289a724d9 --- /dev/null +++ b/tests/virshtestdata/completion-argv-multiple-positional.out @@ -0,0 +1,2 @@ +fv0 + diff --git a/tests/virshtestdata/completion-argv-multiple.out b/tests/virshtestdata/completion-argv-multiple.out new file mode 100644 index 0000000000..4289a724d9 --- /dev/null +++ b/tests/virshtestdata/completion-argv-multiple.out @@ -0,0 +1,2 @@ +fv0 + diff --git a/tests/virshtestdata/completion-command-complete.out b/tests/virshtestdata/completion-command-complete.out new file mode 100644 index 0000000000..40cf7bd7c0 --- /dev/null +++ b/tests/virshtestdata/completion-command-complete.out @@ -0,0 +1,2 @@ +echo + diff --git a/tests/virshtestdata/completion-command.out b/tests/virshtestdata/completion-command.out new file mode 100644 index 0000000000..40cf7bd7c0 --- /dev/null +++ b/tests/virshtestdata/completion-command.out @@ -0,0 +1,2 @@ +echo + diff --git a/tests/virshtestdata/completion.in b/tests/virshtestdata/completion.in new file mode 100644 index 0000000000..cd2594609f --- /dev/null +++ b/tests/virshtestdata/completion.in @@ -0,0 +1,16 @@ +complete -- ech +complete -- echo +complete -- echo '' +complete -- echo --xm --s +complete -- echo --nonexistant-arg --xml +complete -- echo --nonexistant-arg --xml '' +complete -- echo --nonexistant-arg --prefix +complete -- echo --nonexistant-arg --prefix '' +complete -- domstats --domain +complete -- domstats --domain '' +complete -- domstats --domain f +complete -- domstats --domain fc --domain fv +complete -- domstats --domain fv --domain fc '' +complete -- domstats --domain fc fv +complete -- domstate '' +complete -- domstate fv diff --git a/tests/virshtestdata/completion.out b/tests/virshtestdata/completion.out new file mode 100644 index 0000000000..9cceb088e3 --- /dev/null +++ b/tests/virshtestdata/completion.out @@ -0,0 +1,71 @@ +echo + +echo + +--shell +--xml +--split +--err +--prefix +--string + +--shell +--split +--string + +--xml + +--shell +--xml +--split +--err +--prefix +--string + +--prefix + +--shell +--xml +--split +--err +--prefix +--string + +--domain + +test + + + +--state +--cpu-total +--balloon +--vcpu +--interface +--block +--perf +--iothread +--memory +--dirtyrate +--vm +--list-active +--list-inactive +--list-persistent +--list-transient +--list-running +--list-paused +--list-shutoff +--list-other +--raw +--enforce +--backing +--nowait +--domain + + +--domain +--reason + + + +## Exit code: 1 -- 2.44.0

On Fri, Apr 19, 2024 at 15:05:23 +0200, Peter Krempa wrote:
Add both single invocations as well as a script containing the same commands.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> ---
[...]
diff --git a/tests/virshtest.c b/tests/virshtest.c index 14a96f2d35..869ecbb358 100644 --- a/tests/virshtest.c +++ b/tests/virshtest.c @@ -342,6 +342,43 @@ mymain(void) "checkpoint-create test --redefine checkpoint-c2.xml ;" "checkpoint-info test c2");
+ DO_TEST_FULL("completion-command", NULL, VIRSH_CUSTOM, + "complete", "--", "ech"); + DO_TEST_FULL("completion-command-complete", NULL, VIRSH_CUSTOM, + "complete", "--", "echo"); + DO_TEST_FULL("completion-args", NULL, VIRSH_CUSTOM, + "complete", "--", "echo", ""); + DO_TEST_FULL("completion-arg-partial", NULL, VIRSH_CUSTOM, + "complete", "--", "echo", "--xm", "--s"); + DO_TEST_FULL("completion-arg-full-bool", NULL, VIRSH_CUSTOM, + "complete", "--", "echo", "--nonexistant-arg", "--xml"); + DO_TEST_FULL("completion-arg-full-bool-next", NULL, VIRSH_CUSTOM, + "complete", "--", "echo", "--nonexistant-arg", "--xml", ""); + DO_TEST_FULL("completion-arg-full-string", NULL, VIRSH_CUSTOM, + "complete", "--", "echo", "--nonexistant-arg", "--prefix"); + DO_TEST_FULL("completion-arg-full-string-next", NULL, VIRSH_CUSTOM, + "complete", "--", "echo", "--nonexistant-arg", "--prefix", ""); + DO_TEST_FULL("completion-arg-full-argv", NULL, VIRSH_CUSTOM, + "complete", "--", "domstats", "--domain"); + DO_TEST_FULL("completion-arg-full-argv-next", NULL, VIRSH_DEFAULT, + "complete", "--", "domstats", "--domain", ""); + DO_TEST_FULL("completion-argv-multiple", NULL, VIRSH_CUSTOM, + "complete", "--", "domstats", "--domain", "fc", "--domain", "fv"); + DO_TEST_FULL("completion-argv-multiple-next", NULL, VIRSH_CUSTOM, + "complete", "--", "domstats", "--domain", "fv", "--domain", "fc", ""); + DO_TEST_FULL("completion-argv-multiple-positional", NULL, VIRSH_CUSTOM, + "complete", "--", "domstats", "--domain", "fc", "fv"); + DO_TEST_FULL("completion-argv-multiple-positional-next", NULL, VIRSH_CUSTOM, + "complete", "--", "domstats", "--domain", "fc", "fv", ""); + DO_TEST_FULL("completion-arg-positional-empty", NULL, VIRSH_CUSTOM, + "complete", "--", "domstate", ""); + DO_TEST_FULL("completion-arg-positional-partial", NULL, VIRSH_CUSTOM, + "complete", "--", "domstate", "fv"); + DO_TEST_FULL("completion-arg-positional-partial-next", NULL, VIRSH_CUSTOM, + "complete", "--", "domstate", "fv", ""); + + DO_TEST_SCRIPT("completion", NULL, VIRSH_DEFAULT);
Running this trhough CI revealed that I forgot to skip these when readline is not compiled in, which I'll squash into this patch. Additionally two memleaks were found in the original code revealed by this patch, which I've sent patches for.

Currently the code decides which option to complete by looking into the input string and trying to infer it based on whether we are at the end position as we truncate the string to complete to the current cursor position. That basically means that only the last-parsed option will be up for completion. Replace the logic by remembering which is the last option rather than using two different position checks and base the completion decision on that and the actual value of the last argument (see comment). Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/vsh.c | 63 +++++++++++++++++++++++++++-------------------------- tools/vsh.h | 1 + 2 files changed, 33 insertions(+), 31 deletions(-) diff --git a/tools/vsh.c b/tools/vsh.c index 71c0778660..8af6aeaa48 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -1390,6 +1390,7 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser, vshCmd **partial) char *tkdata = NULL; vshCmd *clast = NULL; vshCmdOpt *first = NULL; + vshCmdOpt *last = NULL; const vshCmdDef *cmd = NULL; if (!partial) { @@ -1397,7 +1398,6 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser, vshCmd **partial) } while (1) { - vshCmdOpt *last = NULL; vshCommandToken tk; bool data_only = false; uint64_t opts_need_arg = 0; @@ -1406,6 +1406,7 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser, vshCmd **partial) cmd = NULL; first = NULL; + last = NULL; if (partial) { g_clear_pointer(partial, vshCommandFree); @@ -1589,6 +1590,7 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser, vshCmd **partial) } c->opts = g_steal_pointer(&first); + c->lastopt = last; c->def = cmd; c->next = NULL; @@ -1622,6 +1624,7 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser, vshCmd **partial) tmp = g_new0(vshCmd, 1); tmp->opts = first; + tmp->lastopt = last; tmp->def = cmd; *partial = tmp; @@ -2735,27 +2738,6 @@ vshReadlineOptionsGenerator(const vshCmdDef *cmd, } -static const vshCmdOptDef * -vshReadlineCommandFindOpt(const vshCmd *partial) -{ - const vshCmd *tmp = partial; - - while (tmp) { - const vshCmdOpt *opt = tmp->opts; - - while (opt) { - if (opt->completeThis) - return opt->def; - - opt = opt->next; - } - tmp = tmp->next; - } - - return NULL; -} - - static int vshCompleterFilter(char ***list, const char *text) @@ -2802,7 +2784,6 @@ vshReadlineParse(const char *text, int state) if (!state) { g_autoptr(vshCmd) partial = NULL; const vshCmdDef *cmd = NULL; - const vshCmdOptDef *opt = NULL; g_autofree char *line = g_strdup(rl_line_buffer); g_clear_pointer(&list, g_strfreev); @@ -2828,16 +2809,36 @@ vshReadlineParse(const char *text, int state) cmd = NULL; } - opt = vshReadlineCommandFindOpt(partial); - if (!cmd) { list = vshReadlineCommandGenerator(); - } else if (!opt || opt->type == VSH_OT_BOOL) { - list = vshReadlineOptionsGenerator(cmd, partial); - } else if (opt && opt->completer) { - list = opt->completer(autoCompleteOpaque, - partial, - opt->completer_flags); + } else { + bool complete_argument = false; + + /* attempt completion only when: + - there is an argument + - it has the 'data' field filled + - it has a completer (rules out booleans) + */ + if (partial->lastopt && partial->lastopt->data && partial->lastopt->def->completer) { + /* Furthermore we want to do the completion only at the point of + * user's cursor. This is the case if: + * - value in 'data' is equal to 'text' (last component of the completed command) + * - value in 'data' is a space when 'text' is empty (quirk) + */ + if (STREQ_NULLABLE(partial->lastopt->data, text)) + complete_argument = true; + + if (STREQ_NULLABLE(partial->lastopt->data, " ") && *text == '\0') + complete_argument = true; + } + + if (complete_argument) { + list = partial->lastopt->def->completer(autoCompleteOpaque, + partial, + partial->lastopt->def->completer_flags); + } else { + list = vshReadlineOptionsGenerator(cmd, partial); + } } /* Escape completions, if needed (i.e. argument diff --git a/tools/vsh.h b/tools/vsh.h index a51140eeee..d2426d1e30 100644 --- a/tools/vsh.h +++ b/tools/vsh.h @@ -178,6 +178,7 @@ struct _vshCmdDef { struct _vshCmd { const vshCmdDef *def; /* command definition */ vshCmdOpt *opts; /* list of command arguments */ + vshCmdOpt *lastopt; /* last option of the commandline */ vshCmd *next; /* next command */ bool skipChecks; /* skip validity checks when retrieving opts */ }; -- 2.44.0

In preparation for internal parser refactor introduce new accessors for the VSH_OT_ARGV type which will return a NULL-terminated string list or even a concatenated string for the given argument. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/virsh-checkpoint.c | 10 +-- tools/virsh-domain-monitor.c | 8 +-- tools/virsh-domain.c | 103 +++++++++---------------------- tools/virsh-network.c | 9 +-- tools/virsh-snapshot.c | 9 +-- tools/vsh.c | 114 ++++++++++++++++++++++++----------- tools/vsh.h | 10 ++- 7 files changed, 133 insertions(+), 130 deletions(-) diff --git a/tools/virsh-checkpoint.c b/tools/virsh-checkpoint.c index fea6b4fb4b..972b2f979c 100644 --- a/tools/virsh-checkpoint.c +++ b/tools/virsh-checkpoint.c @@ -226,7 +226,7 @@ cmdCheckpointCreateAs(vshControl *ctl, const char *desc = NULL; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; unsigned int flags = 0; - const vshCmdOpt *opt = NULL; + const char **diskspec = NULL; if (vshCommandOptBool(cmd, "quiesce")) flags |= VIR_DOMAIN_CHECKPOINT_CREATE_QUIESCE; @@ -243,13 +243,15 @@ cmdCheckpointCreateAs(vshControl *ctl, virBufferEscapeString(&buf, "<name>%s</name>\n", name); virBufferEscapeString(&buf, "<description>%s</description>\n", desc); - if (vshCommandOptBool(cmd, "diskspec")) { + if ((diskspec = vshCommandOptArgv(cmd, "diskspec"))) { virBufferAddLit(&buf, "<disks>\n"); virBufferAdjustIndent(&buf, 2); - while ((opt = vshCommandOptArgv(ctl, cmd, opt))) { - if (virshParseCheckpointDiskspec(ctl, &buf, opt->data) < 0) + + for (; *diskspec; diskspec++) { + if (virshParseCheckpointDiskspec(ctl, &buf, *diskspec) < 0) return false; } + virBufferAdjustIndent(&buf, -2); virBufferAddLit(&buf, "</disks>\n"); } diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index 700f3ae094..ef07ace577 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -2096,7 +2096,7 @@ cmdDomstats(vshControl *ctl, const vshCmd *cmd) virDomainStatsRecordPtr *next; bool raw = vshCommandOptBool(cmd, "raw"); int flags = 0; - const vshCmdOpt *opt = NULL; + const char **doms; bool ret = false; virshControl *priv = ctl->privData; @@ -2166,12 +2166,12 @@ cmdDomstats(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool(cmd, "nowait")) flags |= VIR_CONNECT_GET_ALL_DOMAINS_STATS_NOWAIT; - if (vshCommandOptBool(cmd, "domain")) { + if ((doms = vshCommandOptArgv(cmd, "domain"))) { domlist = g_new0(virDomainPtr, 1); ndoms = 1; - while ((opt = vshCommandOptArgv(ctl, cmd, opt))) { - if (!(dom = virshLookupDomainBy(ctl, opt->data, + for (; *doms; doms++) { + if (!(dom = virshLookupDomainBy(ctl, *doms, VIRSH_BYID | VIRSH_BYUUID | VIRSH_BYNAME))) goto cleanup; diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 28d90377a1..50e80689a2 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -5065,15 +5065,15 @@ cmdSchedInfoUpdate(vshControl *ctl, const vshCmd *cmd, { char *set_val = NULL; const char *val = NULL; - const vshCmdOpt *opt = NULL; + const char **opt; virTypedParameterPtr params = NULL; int nparams = 0; int maxparams = 0; int ret = -1; int rv; - while ((opt = vshCommandOptArgv(ctl, cmd, opt))) { - g_autofree char *set_field = g_strdup(opt->data); + for (opt = vshCommandOptArgv(cmd, "set"); opt && *opt; opt++) { + g_autofree char *set_field = g_strdup(*opt); if (!(set_val = strchr(set_field, '='))) { vshError(ctl, "%s", _("Invalid syntax for --set, expecting name=value")); @@ -8248,8 +8248,6 @@ cmdDesc(vshControl *ctl, const vshCmd *cmd) int state; int type; g_autofree char *descArg = NULL; - const vshCmdOpt *opt = NULL; - g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; unsigned int queryflags = 0; @@ -8274,12 +8272,7 @@ cmdDesc(vshControl *ctl, const vshCmd *cmd) else type = VIR_DOMAIN_METADATA_DESCRIPTION; - while ((opt = vshCommandOptArgv(ctl, cmd, opt))) - virBufferAsprintf(&buf, "%s ", opt->data); - - virBufferTrim(&buf, " "); - - descArg = virBufferContentAndReset(&buf); + descArg = g_strdup(vshCommandOptArgvString(cmd, "new-desc")); if (edit || descArg) { g_autofree char *descDom = NULL; @@ -8559,7 +8552,7 @@ cmdSendKey(vshControl *ctl, const vshCmd *cmd) int codeset; unsigned int holdtime = 0; int count = 0; - const vshCmdOpt *opt = NULL; + const char **opt = NULL; int keycode; unsigned int keycodes[VIR_DOMAIN_SEND_KEY_MAX_KEYS]; @@ -8583,15 +8576,15 @@ cmdSendKey(vshControl *ctl, const vshCmd *cmd) return false; } - while ((opt = vshCommandOptArgv(ctl, cmd, opt))) { + for (opt = vshCommandOptArgv(cmd, "keycode"); opt && *opt; opt++) { if (count == VIR_DOMAIN_SEND_KEY_MAX_KEYS) { vshError(ctl, _("too many keycodes")); return false; } - if ((keycode = virshKeyCodeGetInt(opt->data)) < 0) { - if ((keycode = virKeycodeValueFromString(codeset, opt->data)) < 0) { - vshError(ctl, _("invalid keycode: '%1$s'"), opt->data); + if ((keycode = virshKeyCodeGetInt(*opt)) < 0) { + if ((keycode = virKeycodeValueFromString(codeset, *opt)) < 0) { + vshError(ctl, _("invalid keycode: '%1$s'"), *opt); return false; } } @@ -9659,31 +9652,15 @@ static const vshCmdOptDef opts_qemu_monitor_command[] = { }; -static char * -cmdQemuMonitorCommandConcatCmd(vshControl *ctl, - const vshCmd *cmd, - const vshCmdOpt *opt) -{ - g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; - - while ((opt = vshCommandOptArgv(ctl, cmd, opt))) - virBufferAsprintf(&buf, "%s ", opt->data); - - virBufferTrim(&buf, " "); - - return virBufferContentAndReset(&buf); -} - - static char * cmdQemuMonitorCommandQMPWrap(vshControl *ctl, const vshCmd *cmd) { - g_autofree char *fullcmd = cmdQemuMonitorCommandConcatCmd(ctl, cmd, NULL); + const char *fullcmd = vshCommandOptArgvString(cmd, "cmd"); g_autoptr(virJSONValue) fullcmdjson = NULL; g_autofree char *fullargs = NULL; g_autoptr(virJSONValue) fullargsjson = NULL; - const vshCmdOpt *opt = NULL; + const char **opt = NULL; const char *commandname = NULL; g_autoptr(virJSONValue) command = NULL; g_autoptr(virJSONValue) arguments = NULL; @@ -9695,16 +9672,17 @@ cmdQemuMonitorCommandQMPWrap(vshControl *ctl, /* if we've got a JSON object, pass it through */ if (virJSONValueIsObject(fullcmdjson)) - return g_steal_pointer(&fullcmd); + return g_strdup(fullcmd); /* we try to wrap the command and possible arguments into a JSON object, if * we as fall back we pass through what we've got from the user */ - if ((opt = vshCommandOptArgv(ctl, cmd, opt))) - commandname = opt->data; + opt = vshCommandOptArgv(cmd, "cmd"); + commandname = *opt; + opt++; /* now we process arguments similarly to how we've dealt with the full command */ - if ((fullargs = cmdQemuMonitorCommandConcatCmd(ctl, cmd, opt)) && + if ((fullargs = g_strjoinv(" ", (GStrv) opt)) && !(fullargsjson = virJSONValueFromString(fullargs))) { /* Reset the error before adding wrapping. */ vshResetLibvirtError(); @@ -9721,8 +9699,8 @@ cmdQemuMonitorCommandQMPWrap(vshControl *ctl, virBufferAddLit(&buf, "{"); /* opt points to the _ARGV option bit containing the command so we'll * iterate through the arguments now */ - while ((opt = vshCommandOptArgv(ctl, cmd, opt))) - virBufferAsprintf(&buf, "%s,", opt->data); + for (; *opt; opt++) + virBufferAsprintf(&buf, "%s,", *opt); virBufferTrim(&buf, ","); virBufferAddLit(&buf, "}"); @@ -9767,7 +9745,7 @@ cmdQemuMonitorCommand(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool(cmd, "hmp")) { flags |= VIR_DOMAIN_QEMU_MONITOR_COMMAND_HMP; - monitor_cmd = cmdQemuMonitorCommandConcatCmd(ctl, cmd, NULL); + monitor_cmd = g_strdup(vshCommandOptArgvString(cmd, "cmd")); } else { monitor_cmd = cmdQemuMonitorCommandQMPWrap(ctl, cmd); } @@ -10062,26 +10040,16 @@ cmdQemuAgentCommand(vshControl *ctl, const vshCmd *cmd) { g_autoptr(virshDomain) dom = NULL; bool ret = false; - g_autofree char *guest_agent_cmd = NULL; char *result = NULL; int timeout = VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT; int judge = 0; unsigned int flags = 0; - const vshCmdOpt *opt = NULL; - g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; virJSONValue *pretty = NULL; dom = virshCommandOptDomain(ctl, cmd, NULL); if (dom == NULL) goto cleanup; - while ((opt = vshCommandOptArgv(ctl, cmd, opt))) - virBufferAsprintf(&buf, "%s ", opt->data); - - virBufferTrim(&buf, " "); - - guest_agent_cmd = virBufferContentAndReset(&buf); - judge = vshCommandOptInt(ctl, cmd, "timeout", &timeout); if (judge < 0) goto cleanup; @@ -10106,7 +10074,7 @@ cmdQemuAgentCommand(vshControl *ctl, const vshCmd *cmd) goto cleanup; } - result = virDomainQemuAgentCommand(dom, guest_agent_cmd, timeout, flags); + result = virDomainQemuAgentCommand(dom, vshCommandOptArgvString(cmd, "cmd"), timeout, flags); if (!result) goto cleanup; @@ -10158,9 +10126,7 @@ static bool cmdLxcEnterNamespace(vshControl *ctl, const vshCmd *cmd) { g_autoptr(virshDomain) dom = NULL; - const vshCmdOpt *opt = NULL; - g_autofree char **cmdargv = NULL; - size_t ncmdargv = 0; + const char **cmdargv = NULL; pid_t pid; int nfdlist; int *fdlist; @@ -10178,12 +10144,7 @@ cmdLxcEnterNamespace(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool(cmd, "noseclabel")) setlabel = false; - while ((opt = vshCommandOptArgv(ctl, cmd, opt))) { - VIR_EXPAND_N(cmdargv, ncmdargv, 1); - cmdargv[ncmdargv-1] = opt->data; - } - VIR_EXPAND_N(cmdargv, ncmdargv, 1); - cmdargv[ncmdargv - 1] = NULL; + cmdargv = vshCommandOptArgv(cmd, "cmd"); if ((nfdlist = virDomainLxcOpenNamespace(dom, &fdlist, 0)) < 0) return false; @@ -10233,7 +10194,7 @@ cmdLxcEnterNamespace(vshControl *ctl, const vshCmd *cmd) _exit(EXIT_CANCELED); if (pid == 0) { - execv(cmdargv[0], cmdargv); + execv(cmdargv[0], (char **) cmdargv); _exit(errno == ENOENT ? EXIT_ENOENT : EXIT_CANNOT_INVOKE); } @@ -12872,18 +12833,15 @@ static bool cmdDomFSFreeze(vshControl *ctl, const vshCmd *cmd) { g_autoptr(virshDomain) dom = NULL; - const vshCmdOpt *opt = NULL; - g_autofree const char **mountpoints = NULL; + const char **mountpoints = vshCommandOptArgv(cmd, "mountpoint"); size_t nmountpoints = 0; int count = 0; if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; - while ((opt = vshCommandOptArgv(ctl, cmd, opt))) { - VIR_EXPAND_N(mountpoints, nmountpoints, 1); - mountpoints[nmountpoints-1] = opt->data; - } + if (mountpoints) + nmountpoints = g_strv_length((GStrv) mountpoints); if ((count = virDomainFSFreeze(dom, mountpoints, nmountpoints, 0)) < 0) { vshError(ctl, _("Unable to freeze filesystems")); @@ -12913,18 +12871,15 @@ static bool cmdDomFSThaw(vshControl *ctl, const vshCmd *cmd) { g_autoptr(virshDomain) dom = NULL; - const vshCmdOpt *opt = NULL; - g_autofree const char **mountpoints = NULL; + const char **mountpoints = vshCommandOptArgv(cmd, "mountpoint"); size_t nmountpoints = 0; int count = 0; if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; - while ((opt = vshCommandOptArgv(ctl, cmd, opt))) { - VIR_EXPAND_N(mountpoints, nmountpoints, 1); - mountpoints[nmountpoints-1] = opt->data; - } + if (mountpoints) + nmountpoints = g_strv_length((GStrv) mountpoints); if ((count = virDomainFSThaw(dom, mountpoints, nmountpoints, 0)) < 0) { vshError(ctl, _("Unable to thaw filesystems")); diff --git a/tools/virsh-network.c b/tools/virsh-network.c index 24049a66f3..6fcc7fd8ee 100644 --- a/tools/virsh-network.c +++ b/tools/virsh-network.c @@ -388,8 +388,6 @@ cmdNetworkDesc(vshControl *ctl, const vshCmd *cmd) int type; g_autofree char *descArg = NULL; - const vshCmdOpt *opt = NULL; - g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; unsigned int flags = VIR_NETWORK_UPDATE_AFFECT_CURRENT; unsigned int queryflags = 0; @@ -411,12 +409,7 @@ cmdNetworkDesc(vshControl *ctl, const vshCmd *cmd) else type = VIR_NETWORK_METADATA_DESCRIPTION; - while ((opt = vshCommandOptArgv(ctl, cmd, opt))) - virBufferAsprintf(&buf, "%s ", opt->data); - - virBufferTrim(&buf, " "); - - descArg = virBufferContentAndReset(&buf); + descArg = g_strdup(vshCommandOptArgvString(cmd, "new-desc")); if (edit || descArg) { g_autofree char *descNet = NULL; diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c index 415a390786..8b6a950a01 100644 --- a/tools/virsh-snapshot.c +++ b/tools/virsh-snapshot.c @@ -379,7 +379,7 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd) const char *memspec = NULL; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; unsigned int flags = 0; - const vshCmdOpt *opt = NULL; + const char **diskspec; if (vshCommandOptBool(cmd, "no-metadata")) flags |= VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA; @@ -416,11 +416,12 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd) if (memspec && virshParseSnapshotMemspec(ctl, &buf, memspec) < 0) return false; - if (vshCommandOptBool(cmd, "diskspec")) { + if ((diskspec = vshCommandOptArgv(cmd, "diskspec"))) { virBufferAddLit(&buf, "<disks>\n"); virBufferAdjustIndent(&buf, 2); - while ((opt = vshCommandOptArgv(ctl, cmd, opt))) { - if (virshParseSnapshotDiskspec(ctl, &buf, opt->data) < 0) + + for (; *diskspec; diskspec++) { + if (virshParseSnapshotDiskspec(ctl, &buf, *diskspec) < 0) return false; } virBufferAdjustIndent(&buf, -2); diff --git a/tools/vsh.c b/tools/vsh.c index 8af6aeaa48..e9ed7c0794 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -779,6 +779,9 @@ vshCommandOptFree(vshCmdOpt * arg) a = a->next; g_free(tmp->data); + /* 'argv' doesn't own the strings themselves */ + g_free(tmp->argv); + g_free(tmp->argvstr); g_free(tmp); } } @@ -1226,30 +1229,80 @@ vshCommandOptBool(const vshCmd *cmd, const char *name) return vshCommandOpt(cmd, name, &dummy, false) == 1; } + +static vshCmdOpt * +vshCommandOptArgvInternal(const vshCmd *cmd, + const char *name) +{ + vshCmdOpt *first = NULL; + vshCmdOpt *opt; + size_t nargv = 0; + size_t nargv_alloc = 0; + + for (opt = cmd->opts; opt; opt = opt->next) { + if (STRNEQ(opt->def->name, name)) + continue; + + if (!first) { + /* Return existing data if we'we already processed it */ + if (opt->argv) + return opt; + + first = opt; + } + + /* extra NULL terminator */ + VIR_RESIZE_N(first->argv, nargv_alloc, nargv, 2); + first->argv[nargv++] = opt->data; + } + + if (first) + first->argvstr = g_strjoinv(" ", (GStrv) first->argv); + + return first; +} + + /** * vshCommandOptArgv: - * @ctl virtshell control structure - * @cmd command reference - * @opt starting point for the search + * @cmd: command reference + * @name: name of argument * - * Returns the next argv argument after OPT (or the first one if OPT - * is NULL), or NULL if no more are present. + * Returns a NULL terminated list of strings of values passed as argument of + * ARGV argument named @name. The returned string list is owned by @cmd and + * caller must not free or modify it. + */ +const char ** +vshCommandOptArgv(const vshCmd *cmd, + const char *name) +{ + vshCmdOpt *opt = vshCommandOptArgvInternal(cmd, name); + + if (!opt) + return NULL; + + return opt->argv; +} + + +/** + * vshCommandOptArgvString: + * @cmd: command reference + * @name: name of argument * - * Requires that a VSH_OT_ARGV option be last in the - * list of supported options in CMD->def->opts. + * Returns a string containing all values passed as ARGV argument @name + * delimited/concatenated by adding spaces. */ -const vshCmdOpt * -vshCommandOptArgv(vshControl *ctl G_GNUC_UNUSED, const vshCmd *cmd, - const vshCmdOpt *opt) +const char * +vshCommandOptArgvString(const vshCmd *cmd, + const char *name) { - opt = opt ? opt->next : cmd->opts; + vshCmdOpt *opt = vshCommandOptArgvInternal(cmd, name); - while (opt) { - if (opt->def->type == VSH_OT_ARGV) - return opt; - opt = opt->next; - } - return NULL; + if (!opt) + return NULL; + + return opt->argvstr; } @@ -3275,9 +3328,9 @@ cmdEcho(vshControl *ctl, const vshCmd *cmd) bool err = vshCommandOptBool(cmd, "err"); bool split = vshCommandOptBool(cmd, "split"); const char *prefix; - const vshCmdOpt *opt = NULL; g_autofree char *arg = NULL; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; + const char **o; VSH_EXCLUSIVE_OPTIONS_VAR(shell, xml); VSH_EXCLUSIVE_OPTIONS_VAR(shell, split); @@ -3288,8 +3341,8 @@ cmdEcho(vshControl *ctl, const vshCmd *cmd) if (prefix) virBufferAsprintf(&buf, "%s ", prefix); - while ((opt = vshCommandOptArgv(ctl, cmd, opt))) { - const char *curr = opt->data; + for (o = vshCommandOptArgv(cmd, "string"); o && *o; o++) { + const char *curr = *o; if (xml) { virBufferEscapeString(&buf, "%s", curr); @@ -3431,14 +3484,14 @@ bool cmdComplete(vshControl *ctl, const vshCmd *cmd) { const vshClientHooks *hooks = ctl->hooks; - const char *arg = ""; - const vshCmdOpt *opt = NULL; + const char *lastArg = NULL; + const char **args = NULL; g_auto(GStrv) matches = NULL; char **iter; - g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; - if (vshCommandOptStringQuiet(ctl, cmd, "string", &arg) <= 0) - return false; + /* The completer needs also the last component */ + for (args = vshCommandOptArgv(cmd, "string"); args && *args; args++) + lastArg = *args; /* This command is flagged VSH_CMD_FLAG_NOCONNECT because we * need to prevent auth hooks reading any input. Therefore, we @@ -3451,23 +3504,16 @@ cmdComplete(vshControl *ctl, const vshCmd *cmd) if (!(hooks && hooks->connHandler && hooks->connHandler(ctl))) return false; - while ((opt = vshCommandOptArgv(ctl, cmd, opt))) { - if (virBufferUse(&buf) != 0) - virBufferAddChar(&buf, ' '); - virBufferAddStr(&buf, opt->data); - arg = opt->data; - } - vshReadlineInit(ctl); - if (!(rl_line_buffer = virBufferContentAndReset(&buf))) + if (!(rl_line_buffer = g_strdup(vshCommandOptArgvString(cmd, "string")))) rl_line_buffer = g_strdup(""); /* rl_point is current cursor position in rl_line_buffer. * In our case it's at the end of the whole line. */ rl_point = strlen(rl_line_buffer); - if (!(matches = vshReadlineCompletion(arg, 0, 0))) + if (!(matches = vshReadlineCompletion(lastArg, 0, 0))) return false; for (iter = matches; *iter; iter++) { diff --git a/tools/vsh.h b/tools/vsh.h index d2426d1e30..4576c286fe 100644 --- a/tools/vsh.h +++ b/tools/vsh.h @@ -147,6 +147,8 @@ struct _vshCmdOptDef { struct _vshCmdOpt { const vshCmdOptDef *def; /* non-NULL pointer to option definition */ char *data; /* allocated data, or NULL for bool option */ + const char **argv; /* for VSH_OT_ARGV, the list of options */ + char *argvstr; /* space-joined @argv */ bool completeThis; /* true if this is the option user's wishing to autocomplete */ vshCmdOpt *next; @@ -292,8 +294,12 @@ bool vshCommandRun(vshControl *ctl, const vshCmd *cmd); bool vshCommandStringParse(vshControl *ctl, char *cmdstr, vshCmd **partial, size_t point); -const vshCmdOpt *vshCommandOptArgv(vshControl *ctl, const vshCmd *cmd, - const vshCmdOpt *opt); +const char ** +vshCommandOptArgv(const vshCmd *cmd, + const char *name); +const char * +vshCommandOptArgvString(const vshCmd *cmd, + const char *name); bool vshCommandArgvParse(vshControl *ctl, int nargs, char **argv); int vshCommandOptTimeoutToMs(vshControl *ctl, const vshCmd *cmd, int *timeout); -- 2.44.0

Remove the old helpers which were used previously to pick which field to complete. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/virsh.c | 4 ++-- tools/virt-admin.c | 4 ++-- tools/vsh.c | 20 +++----------------- tools/vsh.h | 4 +--- 4 files changed, 8 insertions(+), 24 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 58ebb493fc..a958f2b82f 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -770,7 +770,7 @@ virshParseArgv(vshControl *ctl, int argc, char **argv) ctl->imode = false; if (argc - optind == 1) { vshDebug(ctl, VSH_ERR_INFO, "commands: \"%s\"\n", argv[optind]); - return vshCommandStringParse(ctl, argv[optind], NULL, 0); + return vshCommandStringParse(ctl, argv[optind], NULL); } else { return vshCommandArgvParse(ctl, argc - optind, argv + optind); } @@ -908,7 +908,7 @@ main(int argc, char **argv) if (*ctl->cmdstr) { vshReadlineHistoryAdd(ctl->cmdstr); - if (vshCommandStringParse(ctl, ctl->cmdstr, NULL, 0)) + if (vshCommandStringParse(ctl, ctl->cmdstr, NULL)) vshCommandRun(ctl, ctl->cmd); } VIR_FREE(ctl->cmdstr); diff --git a/tools/virt-admin.c b/tools/virt-admin.c index a996923094..1805618035 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -1331,7 +1331,7 @@ vshAdmParseArgv(vshControl *ctl, int argc, char **argv) ctl->imode = false; if (argc - optind == 1) { vshDebug(ctl, VSH_ERR_INFO, "commands: \"%s\"\n", argv[optind]); - return vshCommandStringParse(ctl, argv[optind], NULL, 0); + return vshCommandStringParse(ctl, argv[optind], NULL); } else { return vshCommandArgvParse(ctl, argc - optind, argv + optind); } @@ -1558,7 +1558,7 @@ main(int argc, char **argv) if (*ctl->cmdstr) { vshReadlineHistoryAdd(ctl->cmdstr); - if (vshCommandStringParse(ctl, ctl->cmdstr, NULL, 0)) + if (vshCommandStringParse(ctl, ctl->cmdstr, NULL)) vshCommandRun(ctl, ctl->cmd); } VIR_FREE(ctl->cmdstr); diff --git a/tools/vsh.c b/tools/vsh.c index e9ed7c0794..1d8aecc41b 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -1430,7 +1430,6 @@ struct _vshCommandParser { char **, bool); /* vshCommandStringGetArg() */ char *pos; - const char *originalLine; size_t point; /* vshCommandArgvGetArg() */ char **arg_pos; @@ -1543,9 +1542,6 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser, vshCmd **partial) arg->data = g_steal_pointer(&tkdata); arg->next = NULL; - if (parser->pos - parser->originalLine == parser->point - 1) - arg->completeThis = true; - if (!first) first = arg; if (last) @@ -1596,9 +1592,6 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser, vshCmd **partial) arg->data = g_steal_pointer(&tkdata); arg->next = NULL; - if (parser->pos - parser->originalLine == parser->point) - arg->completeThis = true; - if (!first) first = arg; if (last) @@ -1812,23 +1805,18 @@ vshCommandStringGetArg(vshControl *ctl, vshCommandParser *parser, char **res, * @ctl virsh control structure * @cmdstr: string to parse * @partial: store partially parsed command here - * @point: position of cursor (rl_point) * * Parse given string @cmdstr as a command and store it under * @ctl->cmd. For readline completion, if @partial is not NULL on * the input then errors in parsing are ignored (because user is * still in progress of writing the command string) and partially * parsed command is stored at *@partial (caller has to free it - * afterwards). Among with @partial, caller must set @point which - * is the position of cursor in @cmdstr (offset, numbered from 1). - * Parser will then set @completeThis attribute to true for the - * vshCmdOpt that appeared under the cursor. + * afterwards). */ bool vshCommandStringParse(vshControl *ctl, char *cmdstr, - vshCmd **partial, - size_t point) + vshCmd **partial) { vshCommandParser parser = { 0 }; @@ -1836,8 +1824,6 @@ vshCommandStringParse(vshControl *ctl, return false; parser.pos = cmdstr; - parser.originalLine = cmdstr; - parser.point = point; parser.getNextArg = vshCommandStringGetArg; return vshCommandParse(ctl, &parser, partial); } @@ -2844,7 +2830,7 @@ vshReadlineParse(const char *text, int state) *(line + rl_point) = '\0'; - vshCommandStringParse(NULL, line, &partial, rl_point); + vshCommandStringParse(NULL, line, &partial); if (partial) { cmd = partial->def; diff --git a/tools/vsh.h b/tools/vsh.h index 4576c286fe..a9c5b674bb 100644 --- a/tools/vsh.h +++ b/tools/vsh.h @@ -149,8 +149,6 @@ struct _vshCmdOpt { char *data; /* allocated data, or NULL for bool option */ const char **argv; /* for VSH_OT_ARGV, the list of options */ char *argvstr; /* space-joined @argv */ - bool completeThis; /* true if this is the option user's wishing to - autocomplete */ vshCmdOpt *next; }; @@ -292,7 +290,7 @@ int vshBlockJobOptionBandwidth(vshControl *ctl, bool vshCommandOptBool(const vshCmd *cmd, const char *name); bool vshCommandRun(vshControl *ctl, const vshCmd *cmd); bool vshCommandStringParse(vshControl *ctl, char *cmdstr, - vshCmd **partial, size_t point); + vshCmd **partial); const char ** vshCommandOptArgv(const vshCmd *cmd, -- 2.44.0

Neither of them is used outside of vsh.c. 'vshCmddefSearch' needed to be rearranged as it was called earlier in vsh.c than it was defined. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/vsh.c | 44 ++++++++++++++++++++++++++------------------ tools/vsh.h | 4 ---- 2 files changed, 26 insertions(+), 22 deletions(-) diff --git a/tools/vsh.c b/tools/vsh.c index 1d8aecc41b..256ba55a83 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -239,6 +239,30 @@ vshReportError(vshControl *ctl) */ static int disconnected; /* we may have been disconnected */ + +/* vshCmddefSearch: + * @cmdname: name of command to find + * + * Looks for @cmdname in the global list of command definitions @cmdGroups and + * returns pointer to the definition struct if the command exists. + */ +static const vshCmdDef * +vshCmddefSearch(const char *cmdname) +{ + const vshCmdGrp *g; + const vshCmdDef *c; + + for (g = cmdGroups; g->name; g++) { + for (c = g->commands; c->name; c++) { + if (STREQ(c->name, cmdname)) + return c; + } + } + + return NULL; +} + + /* Check if the internal command definitions are correct. * None of the errors are to be marked as translatable. */ static int @@ -600,23 +624,7 @@ vshCommandCheckOpts(vshControl *ctl, const vshCmd *cmd, uint64_t opts_required, return -1; } -const vshCmdDef * -vshCmddefSearch(const char *cmdname) -{ - const vshCmdGrp *g; - const vshCmdDef *c; - - for (g = cmdGroups; g->name; g++) { - for (c = g->commands; c->name; c++) { - if (STREQ(c->name, cmdname)) - return c; - } - } - - return NULL; -} - -const vshCmdGrp * +static const vshCmdGrp * vshCmdGrpSearch(const char *grpname) { const vshCmdGrp *g; @@ -629,7 +637,7 @@ vshCmdGrpSearch(const char *grpname) return NULL; } -bool +static bool vshCmdGrpHelp(vshControl *ctl, const vshCmdGrp *grp) { const vshCmdDef *cmd = NULL; diff --git a/tools/vsh.h b/tools/vsh.h index a9c5b674bb..20eaf10022 100644 --- a/tools/vsh.h +++ b/tools/vsh.h @@ -244,10 +244,6 @@ void vshOutputLogFile(vshControl *ctl, int log_level, const char *format, G_GNUC_PRINTF(3, 0); void vshCloseLogFile(vshControl *ctl); -const vshCmdDef *vshCmddefSearch(const char *cmdname); -const vshCmdGrp *vshCmdGrpSearch(const char *grpname); -bool vshCmdGrpHelp(vshControl *ctl, const vshCmdGrp *grp); - int vshCommandOptInt(vshControl *ctl, const vshCmd *cmd, const char *name, int *value) ATTRIBUTE_NONNULL(4) G_GNUC_WARN_UNUSED_RESULT; -- 2.44.0

Refactor the very old opaque logic (using multiple bitmaps) by fully-allocating vshCmdOpt for each possible argument and then filling them as they go rather than allocating them each time after it's parsed. This simplifies the checkers and removes the need to cross-reference multiple arrays. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/vsh.c | 548 +++++++++++++++++++++++----------------------------- tools/vsh.h | 6 +- 2 files changed, 242 insertions(+), 312 deletions(-) diff --git a/tools/vsh.c b/tools/vsh.c index 256ba55a83..30cbf4c0dc 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -476,81 +476,36 @@ vshCmddefCheckInternals(vshControl *ctl, return 0; } -/* Parse the options associated with @cmd, i.e. test whether options are - * required or need an argument and fill the appropriate caller-provided bitmaps - */ -static void -vshCmddefOptParse(const vshCmdDef *cmd, - uint64_t *opts_need_arg, - uint64_t *opts_required) -{ - size_t i; - - *opts_need_arg = 0; - *opts_required = 0; - - if (!cmd->opts) - return; - - for (i = 0; cmd->opts[i].name; i++) { - const vshCmdOptDef *opt = &cmd->opts[i]; - if (opt->type == VSH_OT_BOOL) - continue; - - if (opt->type == VSH_OT_ALIAS) - continue; /* skip the alias option */ - - if (opt->positional || opt->unwanted_positional) - *opts_need_arg |= 1ULL << i; - - if (opt->required) - *opts_required |= 1ULL << i; - } -} - -static vshCmdOptDef helpopt = { - .name = "help", - .type = VSH_OT_BOOL, - .help = N_("print help for this function") -}; - -static const vshCmdOptDef * -vshCmddefGetOption(vshControl *ctl, - const vshCmdDef *cmd, - const char *name, - uint64_t *opts_seen, - size_t *opt_index, - char **optstr, - bool report) +static vshCmdOpt * +vshCmdGetOption(vshControl *ctl, + vshCmd *cmd, + const char *name, + char **optstr, + bool report) { - size_t i; g_autofree char *alias = NULL; + vshCmdOpt *n; - if (STREQ(name, helpopt.name)) - return &helpopt; - - for (i = 0; cmd->opts && cmd->opts[i].name; i++) { - const vshCmdOptDef *opt = &cmd->opts[i]; - - if (STRNEQ(opt->name, name)) + for (n = cmd->opts; n && n->def; n++) { + if (STRNEQ(n->def->name, name)) continue; - if (opt->type == VSH_OT_ALIAS) { + if (n->def->type == VSH_OT_ALIAS) { char *value; /* Two types of replacements: opt->help = "string": straight replacement of name opt->help = "string=value": treat boolean flag as alias of option and its default value */ - alias = g_strdup(opt->help); + alias = g_strdup(n->def->help); name = alias; if ((value = strchr(name, '='))) { *value = '\0'; if (*optstr) { if (report) vshError(ctl, _("invalid '=' after option --%1$s"), - opt->name); + n->def->name); return NULL; } *optstr = g_strdup(value + 1); @@ -558,72 +513,122 @@ vshCmddefGetOption(vshControl *ctl, continue; } - if ((*opts_seen & (1ULL << i)) && opt->type != VSH_OT_ARGV) { + if (n->present && n->def->type != VSH_OT_ARGV) { if (report) vshError(ctl, _("option --%1$s already seen"), name); + return NULL; } - *opts_seen |= 1ULL << i; - *opt_index = i; - return opt; + return n; } /* The 'help' command ignores extra options */ - if (STRNEQ(cmd->name, "help") && report) { + if (STRNEQ(cmd->def->name, "help") && report) { vshError(ctl, _("command '%1$s' doesn't support option --%2$s"), - cmd->name, name); + cmd->def->name, name); } return NULL; } -static const vshCmdOptDef * -vshCmddefGetData(const vshCmdDef *cmd, uint64_t *opts_need_arg, - uint64_t *opts_seen) + +static void +vshCmdOptAssign(vshCmd *cmd, + vshCmdOpt *opt, + const char *val) { - size_t i; - const vshCmdOptDef *opt; + cmd->lastopt = opt; - if (!*opts_need_arg) - return NULL; + opt->present = true; + + switch (opt->def->type) { + case VSH_OT_BOOL: + /* nothing to do */ + break; + + case VSH_OT_STRING: + case VSH_OT_INT: + opt->data = g_strdup(val); + break; + + case VSH_OT_ARGV: + VIR_EXPAND_N(opt->argv, opt->nargv, 2); + /* VIR_EXPAND_N updates count */ + opt->nargv--; + opt->argv[opt->nargv - 1] = g_strdup(val); + /* for completers to work properly we need to also remember the last + * field in 'data' */ + g_clear_pointer(&opt->data, g_free); + opt->data = g_strdup(val); + break; + + case VSH_OT_NONE: + case VSH_OT_ALIAS: + /* impossible code path */ + break; + } +} + + +/** + * vshCmdGetNextPositionalOpt: + * @cmd: command structure + * + * Get next unpopulated positional argument definition. + */ +static vshCmdOpt * +vshCmdGetNextPositionalOpt(const vshCmd *cmd) +{ + vshCmdOpt *n; + + for (n = cmd->opts; n && n->def; n++) { + /* Consider only "positional" options. Tests ensure that boolean options + * don't set these. */ + if (!(n->def->positional || n->def->unwanted_positional)) + continue; - /* Grab least-significant set bit */ - i = __builtin_ffsl(*opts_need_arg) - 1; - opt = &cmd->opts[i]; - if (opt->type != VSH_OT_ARGV) - *opts_need_arg &= ~(1ULL << i); - *opts_seen |= 1ULL << i; - return opt; + /* 'VSH_OT_ARGV' positionals must allow multiple arguments */ + if (n->present && + n->def->type != VSH_OT_ARGV) + continue; + + return n; + } + + return NULL; } + /* * Checks for required options */ static int -vshCommandCheckOpts(vshControl *ctl, const vshCmd *cmd, uint64_t opts_required, - uint64_t opts_seen) +vshCommandCheckOpts(vshControl *ctl, + const vshCmd *cmd) { - const vshCmdDef *def = cmd->def; - size_t i; - - opts_required &= ~opts_seen; - if (!opts_required) - return 0; + vshCmdOpt *n; - for (i = 0; def->opts[i].name; i++) { - if (opts_required & (1ULL << i)) { - const vshCmdOptDef *opt = &def->opts[i]; + for (n = cmd->opts; n && n->def; n++) { + if (!n->present && n->def->required) { + if (n->def->positional) { + vshError(ctl, + _("command '%1$s' requires <%2$s> option"), + cmd->def->name, n->def->name); + } else { + vshError(ctl, + _("command '%1$s' requires --%2$s option"), + cmd->def->name, n->def->name); + } - vshError(ctl, - opt->positional ? - _("command '%1$s' requires <%2$s> option") : - _("command '%1$s' requires --%2$s option"), - def->name, opt->name); + return -1; } } - return -1; + + + return 0; } + static const vshCmdGrp * vshCmdGrpSearch(const char *grpname) { @@ -776,24 +781,6 @@ vshCmddefHelp(const vshCmdDef *def) * Utils for work with runtime commands data * --------------- */ -static void -vshCommandOptFree(vshCmdOpt * arg) -{ - vshCmdOpt *a = arg; - - while (a) { - vshCmdOpt *tmp = a; - - a = a->next; - - g_free(tmp->data); - /* 'argv' doesn't own the strings themselves */ - g_free(tmp->argv); - g_free(tmp->argvstr); - g_free(tmp); - } -} - static void vshCommandFree(vshCmd *cmd) { @@ -801,10 +788,18 @@ vshCommandFree(vshCmd *cmd) while (c) { vshCmd *tmp = c; + vshCmdOpt *n; c = c->next; - vshCommandOptFree(tmp->opts); + for (n = tmp->opts; n && n->def; n++) { + g_free(n->data); + g_strfreev(n->argv); + g_free(n->argvstr); + } + + g_free(tmp->opts); + g_free(tmp); } } @@ -826,40 +821,37 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(vshCmd, vshCommandFree); * is set. No error messages are issued if a value is returned. */ static int -vshCommandOpt(const vshCmd *cmd, const char *name, vshCmdOpt **opt, +vshCommandOpt(const vshCmd *cmd, + const char *name, + vshCmdOpt **opt, bool needData) { - vshCmdOpt *candidate = cmd->opts; - const vshCmdOptDef *valid = cmd->def->opts; - int ret = 0; + vshCmdOpt *n; - /* See if option is valid and/or required. */ *opt = NULL; - while (valid && valid->name) { - if (STREQ(name, valid->name)) - break; - valid++; - } - - if (!cmd->skipChecks) - assert(valid && (!needData || valid->type != VSH_OT_BOOL)); + for (n = cmd->opts; n && n->def; n++) { + if (STRNEQ(name, n->def->name)) + continue; - if (valid && valid->required) - ret = -1; + if (!cmd->skipChecks) + assert(!needData || n->def->type != VSH_OT_BOOL); - /* See if option is present on command line. */ - while (candidate) { - if (STREQ(candidate->def->name, name)) { - *opt = candidate; - ret = 1; - break; + if (n->present) { + *opt = n; + return 1; + } else { + return 0; } - candidate = candidate->next; } - return ret; + + if (!cmd->skipChecks) + assert(false); + + return -1; } + /** * vshCommandOptInt: * @ctl virtshell control structure @@ -1238,39 +1230,6 @@ vshCommandOptBool(const vshCmd *cmd, const char *name) } -static vshCmdOpt * -vshCommandOptArgvInternal(const vshCmd *cmd, - const char *name) -{ - vshCmdOpt *first = NULL; - vshCmdOpt *opt; - size_t nargv = 0; - size_t nargv_alloc = 0; - - for (opt = cmd->opts; opt; opt = opt->next) { - if (STRNEQ(opt->def->name, name)) - continue; - - if (!first) { - /* Return existing data if we'we already processed it */ - if (opt->argv) - return opt; - - first = opt; - } - - /* extra NULL terminator */ - VIR_RESIZE_N(first->argv, nargv_alloc, nargv, 2); - first->argv[nargv++] = opt->data; - } - - if (first) - first->argvstr = g_strjoinv(" ", (GStrv) first->argv); - - return first; -} - - /** * vshCommandOptArgv: * @cmd: command reference @@ -1284,12 +1243,12 @@ const char ** vshCommandOptArgv(const vshCmd *cmd, const char *name) { - vshCmdOpt *opt = vshCommandOptArgvInternal(cmd, name); + vshCmdOpt *opt; - if (!opt) + if (vshCommandOpt(cmd, name, &opt, true) != 1) return NULL; - return opt->argv; + return (const char **) opt->argv; } @@ -1305,11 +1264,14 @@ const char * vshCommandOptArgvString(const vshCmd *cmd, const char *name) { - vshCmdOpt *opt = vshCommandOptArgvInternal(cmd, name); + vshCmdOpt *opt; - if (!opt) + if (vshCommandOpt(cmd, name, &opt, true) != 1) return NULL; + if (!opt->argvstr) + opt->argvstr = g_strjoinv(" ", opt->argv); + return opt->argvstr; } @@ -1444,14 +1406,71 @@ struct _vshCommandParser { char **arg_end; }; + +static vshCmd * +vshCmdNewHelp(const char *name) +{ + vshCmd *c = g_new0(vshCmd, 1); + + c->def = vshCmddefSearch("help"); + + c->opts = g_new0(vshCmdOpt, 2); + c->opts->def = c->def->opts; + c->opts->data = g_strdup(name); + c->opts->present = true; + + return c; +} + + +static vshCmd * +vshCmdNew(vshControl *ctl, + const char *cmdname, + bool report) +{ + g_autoptr(vshCmd) c = g_new0(vshCmd, 1); + const vshCmdOptDef *optdef; + vshCmdOpt *opt; + size_t nopts = 0; + + if (!(c->def = vshCmddefSearch(cmdname))) { + if (report) + vshError(ctl, _("unknown command: '%1$s'"), cmdname); + + return NULL; + } + + /* resolve command alias */ + if (c->def->alias) { + if (!(c->def = vshCmddefSearch(c->def->alias))) { + /* dead code: self-test ensures that the alias exists thus no error reported here */ + return NULL; + } + } + + /* Find number of arguments */ + for (optdef = c->def->opts; optdef && optdef->name; optdef++) + nopts++; + + c->opts = g_new0(vshCmdOpt, nopts + 1); + opt = c->opts; + + /* populate links to definitions */ + for (optdef = c->def->opts; optdef && optdef->name; optdef++) { + opt->def = optdef; + opt++; + } + + return g_steal_pointer(&c); +} + + static bool vshCommandParse(vshControl *ctl, vshCommandParser *parser, vshCmd **partial) { + g_autoptr(vshCmd) cmd = NULL; char *tkdata = NULL; vshCmd *clast = NULL; - vshCmdOpt *first = NULL; - vshCmdOpt *last = NULL; - const vshCmdDef *cmd = NULL; if (!partial) { g_clear_pointer(&ctl->cmd, vshCommandFree); @@ -1460,20 +1479,13 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser, vshCmd **partial) while (1) { vshCommandToken tk; bool data_only = false; - uint64_t opts_need_arg = 0; - uint64_t opts_required = 0; - uint64_t opts_seen = 0; - - cmd = NULL; - first = NULL; - last = NULL; if (partial) { g_clear_pointer(partial, vshCommandFree); } while (1) { - const vshCmdOptDef *opt = NULL; + vshCmdOpt *opt = NULL; tkdata = NULL; tk = parser->getNextArg(ctl, parser, &tkdata, true); @@ -1494,48 +1506,34 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser, vshCmd **partial) } while (tk == VSH_TK_ARG); VIR_FREE(tkdata); break; - } else if (!(cmd = vshCmddefSearch(tkdata))) { - if (!partial) - vshError(ctl, _("unknown command: '%1$s'"), tkdata); - goto syntaxError; /* ... or ignore this command only? */ } - /* aliases need to be resolved to the actual commands */ - if (cmd->alias) { - VIR_FREE(tkdata); - tkdata = g_strdup(cmd->alias); - if (!(cmd = vshCmddefSearch(tkdata))) { - /* self-test ensures that the alias exists */ - vshError(ctl, _("unknown command: '%1$s'"), tkdata); - goto syntaxError; - } - } + if (!(cmd = vshCmdNew(ctl, tkdata, !partial))) + goto syntaxError; - vshCmddefOptParse(cmd, &opts_need_arg, &opts_required); VIR_FREE(tkdata); } else if (data_only) { goto get_data; } else if (tkdata[0] == '-' && tkdata[1] == '-' && g_ascii_isalnum(tkdata[2])) { char *optstr = strchr(tkdata + 2, '='); - size_t opt_index = 0; if (optstr) { *optstr = '\0'; /* convert the '=' to '\0' */ optstr = g_strdup(optstr + 1); } + /* Special case 'help' to ignore all spurious options */ - if (!(opt = vshCmddefGetOption(ctl, cmd, tkdata + 2, - &opts_seen, &opt_index, - &optstr, partial == NULL))) { + if (!(opt = vshCmdGetOption(ctl, cmd, tkdata + 2, + &optstr, partial == NULL))) { VIR_FREE(optstr); - if (STREQ(cmd->name, "help")) + if (STREQ(cmd->def->name, "help")) continue; goto syntaxError; } VIR_FREE(tkdata); - if (opt->type != VSH_OT_BOOL) { + if (opt->def->type != VSH_OT_BOOL) { /* option data */ if (optstr) tkdata = optstr; @@ -1545,33 +1543,23 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser, vshCmd **partial) goto syntaxError; if (tk != VSH_TK_ARG) { if (partial) { - vshCmdOpt *arg = g_new0(vshCmdOpt, 1); - arg->def = opt; - arg->data = g_steal_pointer(&tkdata); - arg->next = NULL; - - if (!first) - first = arg; - if (last) - last->next = arg; - last = arg; + vshCmdOptAssign(cmd, opt, tkdata); + VIR_FREE(tkdata); } else { vshError(ctl, _("expected syntax: --%1$s <%2$s>"), - opt->name, - opt->type == + opt->def->name, + opt->def->type == VSH_OT_INT ? _("number") : _("string")); } goto syntaxError; } - if (opt->type != VSH_OT_ARGV) - opts_need_arg &= ~(1ULL << opt_index); } else { tkdata = NULL; if (optstr) { if (!partial) vshError(ctl, _("invalid '=' after option --%1$s"), - opt->name); + opt->def->name); VIR_FREE(optstr); goto syntaxError; } @@ -1584,85 +1572,52 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser, vshCmd **partial) } else { get_data: /* Special case 'help' to ignore spurious data */ - if (!(opt = vshCmddefGetData(cmd, &opts_need_arg, - &opts_seen)) && - STRNEQ(cmd->name, "help")) { + if (!(opt = vshCmdGetNextPositionalOpt(cmd)) && + STRNEQ(cmd->def->name, "help")) { if (!partial) vshError(ctl, _("unexpected data '%1$s'"), tkdata); goto syntaxError; } } - if (opt) { - /* save option */ - vshCmdOpt *arg = g_new0(vshCmdOpt, 1); - - arg->def = opt; - arg->data = g_steal_pointer(&tkdata); - arg->next = NULL; - if (!first) - first = arg; - if (last) - last->next = arg; - last = arg; + if (opt) { + vshCmdOptAssign(cmd, opt, tkdata); + VIR_FREE(tkdata); if (!partial) vshDebug(ctl, VSH_ERR_INFO, "%s: %s(%s): %s\n", - cmd->name, - opt->name, - opt->type != VSH_OT_BOOL ? _("optdata") : _("bool"), - opt->type != VSH_OT_BOOL ? arg->data : _("(none)")); + cmd->def->name, + opt->def->name, + opt->def->type != VSH_OT_BOOL ? _("optdata") : _("bool"), + opt->def->type != VSH_OT_BOOL ? opt->data : _("(none)")); } } /* command parsed -- allocate new struct for the command */ if (cmd) { - vshCmd *c = g_new0(vshCmd, 1); - vshCmdOpt *tmpopt = first; - - /* if we encountered --help, replace parsed command with - * 'help <cmdname>' */ - for (tmpopt = first; tmpopt; tmpopt = tmpopt->next) { - const vshCmdDef *help; - if (STRNEQ(tmpopt->def->name, "help")) - continue; - - /* the self-test code ensures that help exists */ - if (!(help = vshCmddefSearch("help"))) - break; - - vshCommandOptFree(first); - first = g_new0(vshCmdOpt, 1); - first->def = help->opts; - first->data = g_strdup(cmd->name); - first->next = NULL; + /* if we encountered --help, replace parsed command with 'help <cmdname>' */ + if (cmd->helpOptionSeen) { + vshCmd *helpcmd = vshCmdNewHelp(cmd->def->name); - cmd = help; - opts_required = 0; - opts_seen = 0; - break; + vshCommandFree(cmd); + cmd = helpcmd; } - c->opts = g_steal_pointer(&first); - c->lastopt = last; - c->def = cmd; - c->next = NULL; - if (!partial && - vshCommandCheckOpts(ctl, c, opts_required, opts_seen) < 0) { - vshCommandFree(c); + vshCommandCheckOpts(ctl, cmd) < 0) { + vshCommandFree(cmd); goto syntaxError; } if (partial) { vshCommandFree(*partial); - *partial = c; + *partial = g_steal_pointer(&cmd); } else { if (!ctl->cmd) - ctl->cmd = c; + ctl->cmd = cmd; if (clast) - clast->next = c; - clast = c; + clast->next = cmd; + clast = g_steal_pointer(&cmd); } } @@ -1674,17 +1629,9 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser, vshCmd **partial) syntaxError: if (partial) { - vshCmd *tmp; - - tmp = g_new0(vshCmd, 1); - tmp->opts = first; - tmp->lastopt = last; - tmp->def = cmd; - - *partial = tmp; + *partial = g_steal_pointer(&cmd); } else { g_clear_pointer(&ctl->cmd, vshCommandFree); - vshCommandOptFree(first); } VIR_FREE(tkdata); return false; @@ -2739,43 +2686,24 @@ vshReadlineCommandGenerator(void) static char ** -vshReadlineOptionsGenerator(const vshCmdDef *cmd, - vshCmd *last) +vshReadlineOptionsGenerator(vshCmd *cmd) { - size_t list_index = 0; size_t ret_size = 0; g_auto(GStrv) ret = NULL; + vshCmdOpt *n; - if (!cmd) - return NULL; - - if (!cmd->opts) - return NULL; - - for (list_index = 0; cmd->opts[list_index].name; list_index++) { - const char *name = cmd->opts[list_index].name; - bool exists = false; - vshCmdOpt *opt = last->opts; - + for (n = cmd->opts; n && n->def; n++) { /* Skip aliases, we do not report them in help output either. */ - if (cmd->opts[list_index].type == VSH_OT_ALIAS) + if (n->def->type == VSH_OT_ALIAS) continue; - while (opt) { - if (STREQ(opt->def->name, name) && opt->def->type != VSH_OT_ARGV) { - exists = true; - break; - } - - opt = opt->next; - } - - if (exists) + /* skip already populated single-instance arguments */ + if (n->present && n->def->type != VSH_OT_ARGV) continue; VIR_REALLOC_N(ret, ret_size + 2); - ret[ret_size] = g_strdup_printf("--%s", name); + ret[ret_size] = g_strdup_printf("--%s", n->def->name); ret_size++; /* Terminate the string list properly. */ ret[ret_size] = NULL; @@ -2884,7 +2812,7 @@ vshReadlineParse(const char *text, int state) partial, partial->lastopt->def->completer_flags); } else { - list = vshReadlineOptionsGenerator(cmd, partial); + list = vshReadlineOptionsGenerator(partial); } } diff --git a/tools/vsh.h b/tools/vsh.h index 20eaf10022..66226940ef 100644 --- a/tools/vsh.h +++ b/tools/vsh.h @@ -146,10 +146,11 @@ struct _vshCmdOptDef { */ struct _vshCmdOpt { const vshCmdOptDef *def; /* non-NULL pointer to option definition */ + bool present; /* true if option was present on command line */ char *data; /* allocated data, or NULL for bool option */ - const char **argv; /* for VSH_OT_ARGV, the list of options */ + char **argv; /* for VSH_OT_ARGV, the list of options */ + size_t nargv; char *argvstr; /* space-joined @argv */ - vshCmdOpt *next; }; /* @@ -181,6 +182,7 @@ struct _vshCmd { vshCmdOpt *lastopt; /* last option of the commandline */ vshCmd *next; /* next command */ bool skipChecks; /* skip validity checks when retrieving opts */ + bool helpOptionSeen; /* The '--help' option was seen when persing the command */ }; /* -- 2.44.0

This check was needed due to the use "unsigned long long" as bitmap which was refactored recently. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/vsh.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tools/vsh.c b/tools/vsh.c index 30cbf4c0dc..3cd9202a8a 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -327,11 +327,6 @@ vshCmddefCheckInternals(vshControl *ctl, for (i = 0; cmd->opts[i].name; i++) { const vshCmdOptDef *opt = &cmd->opts[i]; - if (i > 63) { - vshError(ctl, "command '%s' has too many options", cmd->name); - return -1; /* too many options */ - } - if (missingCompleters && !opt->completer) { switch (opt->type) { case VSH_OT_STRING: -- 2.44.0

As we now have a centralized point to assign values to options move the debugging logic there. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/vsh.c | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/tools/vsh.c b/tools/vsh.c index 3cd9202a8a..b04b4f5cd0 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -528,9 +528,11 @@ vshCmdGetOption(vshControl *ctl, static void -vshCmdOptAssign(vshCmd *cmd, +vshCmdOptAssign(vshControl *ctl, + vshCmd *cmd, vshCmdOpt *opt, - const char *val) + const char *val, + bool report) { cmd->lastopt = opt; @@ -539,14 +541,28 @@ vshCmdOptAssign(vshCmd *cmd, switch (opt->def->type) { case VSH_OT_BOOL: /* nothing to do */ + if (report) { + vshDebug(ctl, VSH_ERR_INFO, "%s: %s(bool)\n", + cmd->def->name, opt->def->name); + } break; case VSH_OT_STRING: case VSH_OT_INT: + if (report) { + vshDebug(ctl, VSH_ERR_INFO, "%s: %s(optdata): %s\n", + cmd->def->name, opt->def->name, NULLSTR(val)); + } + opt->data = g_strdup(val); break; case VSH_OT_ARGV: + if (report) { + vshDebug(ctl, VSH_ERR_INFO, "%s: %s(argv: %zu): %s\n", + cmd->def->name, opt->def->name, opt->nargv, NULLSTR(val)); + } + VIR_EXPAND_N(opt->argv, opt->nargv, 2); /* VIR_EXPAND_N updates count */ opt->nargv--; @@ -1538,7 +1554,7 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser, vshCmd **partial) goto syntaxError; if (tk != VSH_TK_ARG) { if (partial) { - vshCmdOptAssign(cmd, opt, tkdata); + vshCmdOptAssign(ctl, cmd, opt, tkdata, !partial); VIR_FREE(tkdata); } else { vshError(ctl, @@ -1576,15 +1592,8 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser, vshCmd **partial) } if (opt) { - vshCmdOptAssign(cmd, opt, tkdata); + vshCmdOptAssign(ctl, cmd, opt, tkdata, !partial); VIR_FREE(tkdata); - - if (!partial) - vshDebug(ctl, VSH_ERR_INFO, "%s: %s(%s): %s\n", - cmd->def->name, - opt->def->name, - opt->def->type != VSH_OT_BOOL ? _("optdata") : _("bool"), - opt->def->type != VSH_OT_BOOL ? opt->data : _("(none)")); } } -- 2.44.0

Refactor the existing logic using two nested loops with a jump into the middle of both with 3 separate places fetching next token to a single loop using a state machine with one centralized place to fetch next tokens and add explanation comments. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/vsh.c | 336 ++++++++++++++++++++++++++++++++-------------------- 1 file changed, 210 insertions(+), 126 deletions(-) diff --git a/tools/vsh.c b/tools/vsh.c index b04b4f5cd0..e0a04593b0 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -1476,171 +1476,255 @@ vshCmdNew(vshControl *ctl, } -static bool -vshCommandParse(vshControl *ctl, vshCommandParser *parser, vshCmd **partial) +static int +vshCmdOptAssignPositional(vshControl *ctl, + vshCmd *cmd, + const char *val, + bool report) { - g_autoptr(vshCmd) cmd = NULL; - char *tkdata = NULL; - vshCmd *clast = NULL; + vshCmdOpt *opt; - if (!partial) { + if (!(opt = vshCmdGetNextPositionalOpt(cmd))) { + /* ignore spurious arguments for 'help' command */ + if (STREQ(cmd->def->name, "help")) + return 0; + + if (report) + vshError(ctl, _("unexpected data '%1$s'"), val); + + return -1; + } + + vshCmdOptAssign(ctl, cmd, opt, val, report); + return 0; +} + + +typedef enum { + VSH_CMD_PARSER_STATE_START, + VSH_CMD_PARSER_STATE_COMMENT, + VSH_CMD_PARSER_STATE_COMMAND, + VSH_CMD_PARSER_STATE_ASSIGN_OPT, + VSH_CMD_PARSER_STATE_POSITIONAL_ONLY, +} vshCommandParserState; + +static bool +vshCommandParse(vshControl *ctl, + vshCommandParser *parser, + vshCmd **partial) +{ + g_autoptr(vshCmd) cmds = NULL; /* linked list of all parsed commands in this session */ + vshCmd *cmds_last = NULL; + g_autoptr(vshCmd) cmd = NULL; /* currently parsed command */ + vshCommandParserState state = VSH_CMD_PARSER_STATE_START; + vshCmdOpt *opt = NULL; + g_autofree char *optionvalue = NULL; + bool report = !partial; + bool ret = false; + + if (partial) { + g_clear_pointer(partial, vshCommandFree); + } else { g_clear_pointer(&ctl->cmd, vshCommandFree); } while (1) { - vshCommandToken tk; - bool data_only = false; + /* previous iteration might have already gotten a value. Store it as the + * token in this iteration */ + g_autofree char *tkdata = g_steal_pointer(&optionvalue); - if (partial) { - g_clear_pointer(partial, vshCommandFree); - } - - while (1) { - vshCmdOpt *opt = NULL; + /* If we have a value already or the option to fill is a boolean we + * don't want to fetch a new token */ + if (!(tkdata || + (opt && opt->def->type == VSH_OT_BOOL))) { + vshCommandToken tk; - tkdata = NULL; - tk = parser->getNextArg(ctl, parser, &tkdata, true); + tk = parser->getNextArg(ctl, parser, &tkdata, report); - if (tk == VSH_TK_ERROR) - goto syntaxError; - if (tk != VSH_TK_ARG) { - VIR_FREE(tkdata); + switch (tk) { + case VSH_TK_ARG: + /* will be handled below */ break; - } - if (cmd == NULL) { - /* first token must be command name or comment */ - if (*tkdata == '#') { - do { - VIR_FREE(tkdata); - tk = parser->getNextArg(ctl, parser, &tkdata, false); - } while (tk == VSH_TK_ARG); - VIR_FREE(tkdata); - break; + case VSH_TK_ERROR: + goto out; + + case VSH_TK_END: + case VSH_TK_SUBCMD_END: + /* The last argument name expects a value, but it's missing */ + if (opt) { + if (partial) { + /* for completion to work we need to also store the + * last token into the last 'opt' */ + vshCmdOptAssign(ctl, cmd, opt, tkdata, report); + } else { + if (opt->def->type == VSH_OT_INT) + vshError(ctl, _("expected syntax: --%1$s <number>"), + opt->def->name); + else + vshError(ctl, _("expected syntax: --%1$s <string>"), + opt->def->name); + + goto out; + } } - if (!(cmd = vshCmdNew(ctl, tkdata, !partial))) - goto syntaxError; - - VIR_FREE(tkdata); - } else if (data_only) { - goto get_data; - } else if (tkdata[0] == '-' && tkdata[1] == '-' && - g_ascii_isalnum(tkdata[2])) { - char *optstr = strchr(tkdata + 2, '='); - - if (optstr) { - *optstr = '\0'; /* convert the '=' to '\0' */ - optstr = g_strdup(optstr + 1); - } + /* command parsed -- allocate new struct for the command */ + if (cmd) { + /* if we encountered --help, replace parsed command with 'help <cmdname>' */ + if (cmd->helpOptionSeen) { + vshCmd *helpcmd = vshCmdNewHelp(cmd->def->name); - /* Special case 'help' to ignore all spurious options */ - if (!(opt = vshCmdGetOption(ctl, cmd, tkdata + 2, - &optstr, partial == NULL))) { - VIR_FREE(optstr); - if (STREQ(cmd->def->name, "help")) - continue; - goto syntaxError; - } - VIR_FREE(tkdata); - - if (opt->def->type != VSH_OT_BOOL) { - /* option data */ - if (optstr) - tkdata = optstr; - else - tk = parser->getNextArg(ctl, parser, &tkdata, partial == NULL); - if (tk == VSH_TK_ERROR) - goto syntaxError; - if (tk != VSH_TK_ARG) { - if (partial) { - vshCmdOptAssign(ctl, cmd, opt, tkdata, !partial); - VIR_FREE(tkdata); - } else { - vshError(ctl, - _("expected syntax: --%1$s <%2$s>"), - opt->def->name, - opt->def->type == - VSH_OT_INT ? _("number") : _("string")); - } - goto syntaxError; - } - } else { - tkdata = NULL; - if (optstr) { - if (!partial) - vshError(ctl, _("invalid '=' after option --%1$s"), - opt->def->name); - VIR_FREE(optstr); - goto syntaxError; + vshCommandFree(cmd); + cmd = helpcmd; } + + if (!partial && + vshCommandCheckOpts(ctl, cmd) < 0) + goto out; + + if (!cmds) + cmds = cmd; + if (cmds_last) + cmds_last->next = cmd; + cmds_last = g_steal_pointer(&cmd); } - } else if (tkdata[0] == '-' && tkdata[1] == '-' && - tkdata[2] == '\0') { - VIR_FREE(tkdata); - data_only = true; - continue; - } else { - get_data: - /* Special case 'help' to ignore spurious data */ - if (!(opt = vshCmdGetNextPositionalOpt(cmd)) && - STRNEQ(cmd->def->name, "help")) { - if (!partial) - vshError(ctl, _("unexpected data '%1$s'"), tkdata); - goto syntaxError; + + + /* everything parsed */ + if (tk == VSH_TK_END) { + ret = true; + goto out; } - } - if (opt) { - vshCmdOptAssign(ctl, cmd, opt, tkdata, !partial); - VIR_FREE(tkdata); + /* after processing the command we need to start over again to + * fetch another token */ + state = VSH_CMD_PARSER_STATE_START; + continue; } } - /* command parsed -- allocate new struct for the command */ - if (cmd) { - /* if we encountered --help, replace parsed command with 'help <cmdname>' */ - if (cmd->helpOptionSeen) { - vshCmd *helpcmd = vshCmdNewHelp(cmd->def->name); + /* at this point we know that @tkdata is an argument */ + switch (state) { + case VSH_CMD_PARSER_STATE_START: + if (*tkdata == '#') { + state = VSH_CMD_PARSER_STATE_COMMENT; + } else { + state = VSH_CMD_PARSER_STATE_COMMAND; + + if (!(cmd = vshCmdNew(ctl, tkdata, !partial))) + goto out; + } + + break; + + case VSH_CMD_PARSER_STATE_COMMENT: + /* continue eating tokens until end of line or end of input */ + state = VSH_CMD_PARSER_STATE_COMMENT; + break; + + case VSH_CMD_PARSER_STATE_COMMAND: { + /* parsing individual options for the command. There are following options: + * --option + * --option value + * --option=value + * --aliasoptionwithvalue (value is part of the alias definition) + * value + * -- (terminate accepting '--option', fill only positional args) + */ + const char *optionname = tkdata + 2; + char *sep; - vshCommandFree(cmd); - cmd = helpcmd; + if (!STRPREFIX(tkdata, "--")) { + if (vshCmdOptAssignPositional(ctl, cmd, tkdata, report) < 0) + goto out; + break; } - if (!partial && - vshCommandCheckOpts(ctl, cmd) < 0) { - vshCommandFree(cmd); - goto syntaxError; + if (STREQ(tkdata, "--")) { + state = VSH_CMD_PARSER_STATE_POSITIONAL_ONLY; + break; + } + + if ((sep = strchr(optionname, '='))) { + *(sep++) = '\0'; + + /* 'optionvalue' has lifetime until next iteration */ + optionvalue = g_strdup(sep); } - if (partial) { - vshCommandFree(*partial); - *partial = g_steal_pointer(&cmd); + /* lookup the option. Note that vshCmdGetOption also resolves aliases + * and thus the value possibly contained in the alias */ + if (!(opt = vshCmdGetOption(ctl, cmd, optionname, &optionvalue, report))) { + if (STRNEQ(cmd->def->name, "help")) + goto out; + + /* ignore spurious arguments for 'help' command */ + g_clear_pointer(&optionvalue, g_free); + state = VSH_CMD_PARSER_STATE_COMMAND; } else { - if (!ctl->cmd) - ctl->cmd = cmd; - if (clast) - clast->next = cmd; - clast = g_steal_pointer(&cmd); + state = VSH_CMD_PARSER_STATE_ASSIGN_OPT; } } + break; + + case VSH_CMD_PARSER_STATE_ASSIGN_OPT: + /* Parameter for a boolean was passed via --boolopt=val */ + if (tkdata && opt->def->type == VSH_OT_BOOL) { + if (report) + vshError(ctl, _("invalid '=' after option --%1$s"), + opt->def->name); + goto out; + } - if (tk == VSH_TK_END) + vshCmdOptAssign(ctl, cmd, opt, tkdata, report); + opt = NULL; + state = VSH_CMD_PARSER_STATE_COMMAND; break; + + case VSH_CMD_PARSER_STATE_POSITIONAL_ONLY: + state = VSH_CMD_PARSER_STATE_POSITIONAL_ONLY; + + if (vshCmdOptAssignPositional(ctl, cmd, tkdata, report) < 0) + goto out; + break; + } } - return true; + out: - syntaxError: if (partial) { - *partial = g_steal_pointer(&cmd); + /* When parsing a command for command completion, the last processed + * command or the one being currently parsed */ + if (cmd) { + *partial = g_steal_pointer(&cmd); + } else if (cmds == cmds_last) { + *partial = g_steal_pointer(&cmds); + } else { + /* break the last command out of the linked list and let the rest be freed */ + vshCmd *nc; + + for (nc = cmds; nc; nc = nc->next) { + if (nc->next == cmds_last) { + nc->next = NULL; + break; + } + } + + *partial = cmds_last; + } } else { - g_clear_pointer(&ctl->cmd, vshCommandFree); + /* for normal command parsing use the whole parsed command list, but + * only on success */ + if (ret == true) { + ctl->cmd = g_steal_pointer(&cmds); + } } - VIR_FREE(tkdata); - return false; + + return ret; } + /* -------------------- * Command argv parsing * -------------------- -- 2.44.0

On 4/19/24 15:05, Peter Krempa wrote:
This series refactors the commandline parser in order to use easier to understand/maintain logic.
Peter Krempa (13): meson: tests: Add 'virsh' as dependency of 'virshtest' tools: Rename vshCommandOptStringReq to vshCommandOptString vsh: Fix 'stdin' closing in 'cmdComplete' vsh: Add a VSH_OT_STRING argument for 'virsh echo' virshtest: Add test cases for command completion helper vsh: Rework logic for picking which argument is to be completed virsh: Introduce new 'VSH_OT_ARGV' accessors vsh: Remove unused infrastructure for command completion vsh: Unexport command lookup helpers 'vshCmddefSearch', 'vshCmdGrpSearch', 'vshCmdGrpHelp' vsh: Refactor parsed option and command assignment vshCmddefCheckInternals: Remove check for "too many options" vsh: Move option assignment debugging from vshCommandParse to vshCmdOptAssign vsh: Refactor logic in vshCommandParse
tests/meson.build | 4 +- tests/virshtest.c | 37 + .../completion-arg-full-argv-next.out | 2 + .../completion-arg-full-argv.out | 2 + .../completion-arg-full-bool-next.out | 7 + .../completion-arg-full-bool.out | 2 + .../completion-arg-full-string-next.out | 7 + .../completion-arg-full-string.out | 2 + .../virshtestdata/completion-arg-partial.out | 4 + .../completion-arg-positional-empty.out | 3 + ...completion-arg-positional-partial-next.out | 2 + .../completion-arg-positional-partial.out | 2 + tests/virshtestdata/completion-args.out | 7 + .../completion-argv-multiple-next.out | 25 + ...mpletion-argv-multiple-positional-next.out | 25 + .../completion-argv-multiple-positional.out | 2 + .../completion-argv-multiple.out | 2 + .../completion-command-complete.out | 2 + tests/virshtestdata/completion-command.out | 2 + tests/virshtestdata/completion.in | 16 + tests/virshtestdata/completion.out | 71 ++ tools/virsh-backup.c | 4 +- tools/virsh-checkpoint.c | 18 +- tools/virsh-completer-domain.c | 2 +- tools/virsh-completer-host.c | 8 +- tools/virsh-domain-event.c | 2 +- tools/virsh-domain-monitor.c | 18 +- tools/virsh-domain.c | 351 +++--- tools/virsh-host.c | 40 +- tools/virsh-interface.c | 6 +- tools/virsh-network.c | 35 +- tools/virsh-nodedev.c | 30 +- tools/virsh-nwfilter.c | 8 +- tools/virsh-pool.c | 64 +- tools/virsh-secret.c | 10 +- tools/virsh-snapshot.c | 23 +- tools/virsh-util.c | 2 +- tools/virsh-volume.c | 29 +- tools/virsh.c | 6 +- tools/virt-admin.c | 26 +- tools/vsh.c | 1012 +++++++++-------- tools/vsh.h | 27 +- 42 files changed, 1103 insertions(+), 844 deletions(-) create mode 100644 tests/virshtestdata/completion-arg-full-argv-next.out create mode 100644 tests/virshtestdata/completion-arg-full-argv.out create mode 100644 tests/virshtestdata/completion-arg-full-bool-next.out create mode 100644 tests/virshtestdata/completion-arg-full-bool.out create mode 100644 tests/virshtestdata/completion-arg-full-string-next.out create mode 100644 tests/virshtestdata/completion-arg-full-string.out create mode 100644 tests/virshtestdata/completion-arg-partial.out create mode 100644 tests/virshtestdata/completion-arg-positional-empty.out create mode 100644 tests/virshtestdata/completion-arg-positional-partial-next.out create mode 100644 tests/virshtestdata/completion-arg-positional-partial.out create mode 100644 tests/virshtestdata/completion-args.out create mode 100644 tests/virshtestdata/completion-argv-multiple-next.out create mode 100644 tests/virshtestdata/completion-argv-multiple-positional-next.out create mode 100644 tests/virshtestdata/completion-argv-multiple-positional.out create mode 100644 tests/virshtestdata/completion-argv-multiple.out create mode 100644 tests/virshtestdata/completion-command-complete.out create mode 100644 tests/virshtestdata/completion-command.out create mode 100644 tests/virshtestdata/completion.in create mode 100644 tests/virshtestdata/completion.out
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Michal Prívozník
-
Peter Krempa