From: Daniel Shahaf <d.s@daniel.shahaf.name>
To: Marko Myllynen <myllynen@redhat.com>
Cc: zsh-workers@zsh.org
Subject: Re: _libvirt, $opt_args, and noglob
Date: Wed, 7 Sep 2016 22:42:07 +0000 [thread overview]
Message-ID: <20160907224207.GB7775@fujitsu.shahaf.local2> (raw)
In-Reply-To: <23031e84-df6c-90c7-b5c0-9d6eb157dd61@redhat.com>
See the penultimate paragraph for a question about whether ${(q)} should
quote leading '=' signs to guard against the EQUALS option.
Marko Myllynen wrote on Wed, Sep 07, 2016 at 12:53:13 +0300:
> Hi,
>
> On 2016-09-07 02:46, Daniel Shahaf wrote:
> > Daniel Shahaf wrote on Sun, Sep 04, 2016 at 18:26:22 +0000:
> >> In the special case of _libvirt, simply adding noglob to the
> >> _call_program arguments would probably fix this.
> >
> > Done, but the last line touches the sudo invocation from Oliver's
> > gain-privileges patch. Oliver: shall I wait with committing this until
> > you've committed gain-privileges?
> >
> > @@ -224,7 +224,7 @@ case $state in
> > local srv ; (( ${(k)words[(I)--server]} > 0 )) && srv=${words[1+${(k)words[(I)--server]}]}
> > [[ -z $srv ]] && return 1
> > [[ -n ${srv//[[:alnum:]]} ]] && return 1
> > - _wanted clients expl client compadd ${=${${(f):-"$(sudo virt-admin ${(Q)conn_opt} srv-clients-list --server $srv 2>/dev/null)"}/ [a-z]*}//[^0-9]} && return 0
> > + _wanted clients expl client compadd ${=${${(f):-"$(noglob sudo virt-admin ${(Q)conn_opt} srv-clients-list --server $srv 2>/dev/null)"}/ [a-z]*}//[^0-9]} && return 0
> > fi
> > [[ -z $_cache_virt_admin_cmd_opts[$cmd] ]] && \
> > _cache_virt_admin_cmd_opts[$cmd]=${(M)${${${${=${(f)"$(_call_program virt-admin virt-admin help $cmd 2>&1)"}}/\[}/\]}/\;}:#-[-0-9A-Za-z]*}
> > ]]]
>
> Do we need any of (Q)'s there any more after your addition of:
>
> uri=${uri//(#m)\\([\\:])/${MATCH[2]}} # opt_args elements are colon-escaped
>
> Based on a quick test looks like they could be dropped (or
> perhaps even changed to (q))?
_call_program expects its argument to be (q)'d. (The interface is like
'eval', not like execve(2).)
The values of $opt_args are *already* (q)'d. If the command line is
«virsh -c foo\\bar» then the value of $pot_args[-c] includes two
backslashes, not one as virsh(1)'s argv[2] would see.¹
Therefore, I think the correct way to pass values derived from $opt_args
— this includes $conn_opt — to _call_program is to use neither (q) nor (Q).
That is the general answer. For the specific case of $conn_opt, the URI
format validator ensures it has nothing that needs quoting, apart from
'?' and '=', and the patch in the grandparent message removes the need
for '?' to be quoted. However, there's no easy to to quote '=' when
it's the first character of a word, and _call_program does honour the
EQUALS option:
.
% _f() { compadd - $(_call_program x 'echo =true') }
% f <TAB>
<becomes>
% f /bin/true
Again, this is harmless as far as _libvirt is concerned, but it might
not be so in other callsites. I think the right general solution to
this would be to write ${(q)${(Q)conn_opts}} in the _call_program
invocation, and teach the (q) flag to escape leading equals signs.
Anyway, back to Marko's question: I assume even if the EQUALS option did
affect the command executed, that wouldn't cause any harm, so I think
the primary reason to get _libvirt's quoting right is in case somebody
ever uses _libvirt as a basis for some other completion function.
That's part of why I committed the (b) patch, too.
Cheers,
Daniel
¹ This assumes that the word being completed, $CURRENT, is not the
argument to -c, in which cae $opt_args[-c] could have an opening quote
but no matching closing quote.
prev parent reply other threads:[~2016-09-07 22:43 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-04 18:26 Daniel Shahaf
2016-09-06 23:46 ` Daniel Shahaf
2016-09-07 9:53 ` Marko Myllynen
2016-09-07 22:42 ` Daniel Shahaf [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160907224207.GB7775@fujitsu.shahaf.local2 \
--to=d.s@daniel.shahaf.name \
--cc=myllynen@redhat.com \
--cc=zsh-workers@zsh.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/zsh/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).