qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Markus Armbruster <armbru@redhat.com>, Lin Ma <lma@suse.com>
Cc: mst@redhat.com, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH] qdev: Workaround for an issue caused by noglob option of shell for -device ?
Date: Mon, 27 Jun 2016 08:31:58 -0600	[thread overview]
Message-ID: <577138DE.1090106@redhat.com> (raw)
In-Reply-To: <8760svcffe.fsf@dusky.pond.sub.org>

[-- Attachment #1: Type: text/plain, Size: 2001 bytes --]

On 06/27/2016 12:29 AM, Markus Armbruster wrote:
> Lin Ma <lma@suse.com> writes:
> 
>> If there is any filename in current path matching wildcard ?, This filename will
>> be passed into -device ? while 'pathname expansion' isn't disabled by noglob of
>> shell built-in option, qemu reports error in this case.

And if there is more than one such file, then the glob expands to all
such files, turning '-device ?' into '-device a b', which your patch
does not handle.  You CANNOT reliably detect if a glob was expanded, so
it is pointless to try.  (At one point, glibc+bash had a tentative patch
that would set an environment variable according to the pre-glob command
line passed through the shell, so that the child app could reverse
engineer what globs had been expanded, but it was discarded YEARS ago as
not worth the overhead).

>>
>> Signed-off-by: Lin Ma <lma@suse.com>
>> ---
>>  qdev-monitor.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/qdev-monitor.c b/qdev-monitor.c
>> index e19617f..f6443cd 100644
>> --- a/qdev-monitor.c
>> +++ b/qdev-monitor.c
>> @@ -257,6 +257,11 @@ int qdev_device_help(QemuOpts *opts)
>>      DevicePropertyInfoList *prop;
>>  
>>      driver = qemu_opt_get(opts, "driver");
>> +
>> +    if (driver && access(driver, F_OK == 0) && strlen(driver) == 1) {
>> +        driver = "?";
>> +    }

NACK.

>> +
>>      if (driver && is_help_option(driver)) {
>>          qdev_print_devinfos(false);
>>          return 1;
> 
> I'm afraid this would silently interpret any one-character driver name
> as a request for help when a file with this name exists.  Not a good
> idea.  Example: "-device .".
> 
> "-device ?" and similar use of "?" elsewhere is deprecated.  Please use
> "-device help".

Agreed - we invented 'help' as a synonym for '?' precisely because '?'
can't be worked around.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

      reply	other threads:[~2016-06-27 14:32 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-26 11:53 [Qemu-devel] [PATCH] qdev: Workaround for an issue caused by noglob option of shell for -device ? Lin Ma
2016-06-27  6:29 ` Markus Armbruster
2016-06-27 14:31   ` Eric Blake [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=577138DE.1090106@redhat.com \
    --to=eblake@redhat.com \
    --cc=armbru@redhat.com \
    --cc=lma@suse.com \
    --cc=mst@redhat.com \
    --cc=qemu-devel@nongnu.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.
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).