diff mbox

qdev: Workaround for an issue caused by noglob option of shell for -device ?

Message ID 1466941986-4587-1-git-send-email-lma@suse.com
State New
Headers show

Commit Message

Lin Ma June 26, 2016, 11:53 a.m. UTC
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.

Signed-off-by: Lin Ma <lma@suse.com>
---
 qdev-monitor.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Markus Armbruster June 27, 2016, 6:29 a.m. UTC | #1
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.
>
> 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 = "?";
> +    }
> +
>      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".
Eric Blake June 27, 2016, 2:31 p.m. UTC | #2
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.
diff mbox

Patch

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 = "?";
+    }
+
     if (driver && is_help_option(driver)) {
         qdev_print_devinfos(false);
         return 1;