From nobody Tue Feb 10 04:16:46 2026 Delivered-To: importer@patchew.org Received-SPF: none (zohomail.com: 8.43.85.245 is neither permitted nor denied by domain of lists.libvirt.org) client-ip=8.43.85.245; envelope-from=devel-bounces@lists.libvirt.org; helo=lists.libvirt.org; Authentication-Results: mx.zohomail.com; spf=none (zohomail.com: 8.43.85.245 is neither permitted nor denied by domain of lists.libvirt.org) smtp.mailfrom=devel-bounces@lists.libvirt.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.libvirt.org (lists.libvirt.org [8.43.85.245]) by mx.zohomail.com with SMTPS id 1710510613286291.48890006027455; Fri, 15 Mar 2024 06:50:13 -0700 (PDT) Received: by lists.libvirt.org (Postfix, from userid 996) id 1AF231A88; Fri, 15 Mar 2024 09:50:12 -0400 (EDT) Received: from lists.libvirt.org (localhost [IPv6:::1]) by lists.libvirt.org (Postfix) with ESMTP id 99D4A1C04; Fri, 15 Mar 2024 09:44:59 -0400 (EDT) Received: by lists.libvirt.org (Postfix, from userid 996) id 8533C1A8E; Fri, 15 Mar 2024 09:44:48 -0400 (EDT) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by lists.libvirt.org (Postfix) with ESMTPS id 926B91A8E for ; Fri, 15 Mar 2024 09:44:34 -0400 (EDT) Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-568-YxU2Z3IKNmWVvOAKhuwGiw-1; Fri, 15 Mar 2024 09:44:33 -0400 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.rdu2.redhat.com [10.11.54.8]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id BC109185A78E for ; Fri, 15 Mar 2024 13:44:31 +0000 (UTC) Received: from speedmetal.lan (unknown [10.45.242.5]) by smtp.corp.redhat.com (Postfix) with ESMTP id 35C12C1576F for ; Fri, 15 Mar 2024 13:44:31 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on lists.libvirt.org X-Spam-Level: X-Spam-Status: No, score=-0.8 required=5.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.4 X-MC-Unique: YxU2Z3IKNmWVvOAKhuwGiw-1 From: Peter Krempa To: devel@lists.libvirt.org Subject: [PATCH 07/28] vsh: Introduce tool to find unwanted positional arguments to 'self-test' Date: Fri, 15 Mar 2024 14:44:02 +0100 Message-ID: In-Reply-To: References: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.8 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Message-ID-Hash: Q3MTVY2Y4JHJ7YYI7TWLWPKTGRX2OYXS X-Message-ID-Hash: Q3MTVY2Y4JHJ7YYI7TWLWPKTGRX2OYXS X-MailFrom: pkrempa@redhat.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; header-match-config-1; header-match-config-2; header-match-config-3; header-match-devel.lists.libvirt.org-0; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; suspicious-header X-Mailman-Version: 3.2.2 Precedence: list List-Id: Development discussions about the libvirt library & tools Archived-At: List-Archive: List-Help: List-Post: List-Subscribe: List-Unsubscribe: Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-ZM-MESSAGEID: 1710510614277100001 While the virsh option definitions specify (either explicitly after recent refactors, or implicitly before) whether an argument is positional or not, the actual parser is way more lax and actually and allows also arguments which were considered/documented as non-positional to be filled positionally unless VSH_OFLAG_REQ_OPT is used in the flags. This creates situations such as 'snapshot-create-as' which has the following docs: SYNOPSIS snapshot-create-as [--name ] [--description ] [--print-xml] [--no-metadata] [--halt] [--disk-only] [--reuse-external] [--quiesce] [--atomic] [--live] [--validate] [--memspec ] [[--diskspec] ]... Thus showing as if '--name' and '--description' required the option, but in fact the following happens when only positionals are passed: $ virsh snapshot-create-as --print-xml 1 2 3 4 5 2 3 In the above example e.g. '--memspec' is not populated. This disconnect makes it impossible to refactor the parser itself and allows users to write buggy interactions with virsh. In order to address this we'll be annotating every single of these unwanted positional options as such so that this doesn't happen in the future, while still preserving the quirk in the parser. This patch introduces a tool which outputs list of options which are not marked as positional but are lacking the VSH_OFLAG_REQ_OPT flag. This tool will be removed once all the offenders found by it will be addressed. Signed-off-by: Peter Krempa --- tools/vsh.c | 41 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/tools/vsh.c b/tools/vsh.c index a4235905dc..96d6a5c238 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -244,11 +244,13 @@ static int disconnected; /* we may have been disconne= cted */ static int vshCmddefCheckInternals(vshControl *ctl, const vshCmdDef *cmd, - bool missingCompleters) + bool missingCompleters, + int brokenPositionals) { size_t i; bool seenOptionalOption =3D false; g_auto(virBuffer) complbuf =3D VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) posbuf =3D VIR_BUFFER_INITIALIZER; /* in order to perform the validation resolve the alias first */ if (cmd->alias) { @@ -325,6 +327,28 @@ vshCmddefCheckInternals(vshControl *ctl, } } + if (brokenPositionals >=3D 0) { + switch (opt->type) { + case VSH_OT_INT: + case VSH_OT_STRING: + case VSH_OT_ARGV: + if (brokenPositionals =3D=3D 0 || + brokenPositionals =3D=3D opt->type) { + if (!(opt->flags & VSH_OFLAG_REQ_OPT) && !opt->positio= nal) + virBufferStrcat(&posbuf, opt->name, ", ", NULL); + } + break; + + case VSH_OT_BOOL: + /* only name is completed */ + /* no point in completing numbers */ + case VSH_OT_ALIAS: + /* alias is handled in the referenced command */ + case VSH_OT_NONE: + break; + } + } + /* require that positional non-argv options are required */ if (opt->positional && !opt->required && opt->type !=3D VSH_OT_ARG= V) { vshError(ctl, "positional argument '%s' of command '%s' must b= e required", @@ -427,10 +451,15 @@ vshCmddefCheckInternals(vshControl *ctl, } virBufferTrim(&complbuf, ", "); + virBufferTrim(&posbuf, ", "); if (missingCompleters && virBufferUse(&complbuf) > 0) vshPrintExtra(ctl, "%s: %s\n", cmd->name, virBufferCurrentContent(= &complbuf)); + if (virBufferUse(&posbuf)) { + vshPrintExtra(ctl, "%s: %s\n", cmd->name, virBufferCurrentContent(= &posbuf)); + } + return 0; } @@ -3336,6 +3365,11 @@ const vshCmdOptDef opts_selftest[] =3D { .type =3D VSH_OT_BOOL, .help =3D N_("output help for each command") }, + {.name =3D "broken-positionals", + .type =3D VSH_OT_INT, + .flags =3D VSH_OFLAG_REQ_OPT, + .help =3D N_("debug positional args") + }, {.name =3D NULL} }; const vshCmdInfo info_selftest =3D { @@ -3350,6 +3384,9 @@ cmdSelfTest(vshControl *ctl, const vshCmd *cmd) const vshCmdDef *def; bool completers =3D vshCommandOptBool(cmd, "completers-missing"); bool dumphelp =3D vshCommandOptBool(cmd, "dump-help"); + int brokenPositionals =3D -1; + + ignore_value(vshCommandOptInt(ctl, cmd, "broken-positionals", &brokenP= ositionals)); for (grp =3D cmdGroups; grp->name; grp++) { for (def =3D grp->commands; def->name; def++) { @@ -3357,7 +3394,7 @@ cmdSelfTest(vshControl *ctl, const vshCmd *cmd) if (dumphelp && !def->alias) vshCmddefHelp(def); - if (vshCmddefCheckInternals(ctl, def, completers) < 0) + if (vshCmddefCheckInternals(ctl, def, completers, brokenPositi= onals) < 0) return false; } } --=20 2.44.0 _______________________________________________ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-leave@lists.libvirt.org