Patchwork fix '-cpu ?' Segfault

login
register
mail settings
Submitter Eduardo Habkost
Date March 21, 2012, 12:33 p.m.
Message ID <1332333220-31420-1-git-send-email-ehabkost@redhat.com>
Download mbox | patch
Permalink /patch/147971/
State New
Headers show

Comments

Eduardo Habkost - March 21, 2012, 12:33 p.m.
Fix stupid copy&paste mistake at commit
ecf40beae7dcbb057d4f115207f9d8276832a774: I moved code around but kept
"optarg" on the cpu_list() call.

Reported-by: Jiri Denemark <jdenemar@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 vl.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Stefan Hajnoczi - March 21, 2012, 1:24 p.m.
On Wed, Mar 21, 2012 at 09:33:40AM -0300, Eduardo Habkost wrote:
> Fix stupid copy&paste mistake at commit
> ecf40beae7dcbb057d4f115207f9d8276832a774: I moved code around but kept
> "optarg" on the cpu_list() call.
> 
> Reported-by: Jiri Denemark <jdenemar@redhat.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  vl.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

Thanks, applied to the trivial patches tree:
https://github.com/stefanha/qemu/commits/trivial-patches

Stefan
Stefan Berger - March 22, 2012, 1:28 p.m.
On 03/21/2012 08:33 AM, Eduardo Habkost wrote:
> Fix stupid copy&paste mistake at commit
> ecf40beae7dcbb057d4f115207f9d8276832a774: I moved code around but kept
> "optarg" on the cpu_list() call.
>
> Reported-by: Jiri Denemark<jdenemar@redhat.com>
> Signed-off-by: Eduardo Habkost<ehabkost@redhat.com>
> ---
>   vl.c |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index 112b0e0..0fccf50 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3196,7 +3196,7 @@ int main(int argc, char **argv, char **envp)
>       cpudef_init();
>
>       if (cpu_model&&  *cpu_model == '?') {
> -        list_cpus(stdout,&fprintf, optarg);
> +        list_cpus(stdout,&fprintf, cpu_model);
>           exit(0);
>       }
>
Does -cpu ? actually work then? I only see the list of cpus when using 
-cpu ?_ for example.

    Stefan
Andreas Färber - March 22, 2012, 1:46 p.m.
Am 22.03.2012 14:28, schrieb Stefan Berger:
> On 03/21/2012 08:33 AM, Eduardo Habkost wrote:
>> Fix stupid copy&paste mistake at commit
>> ecf40beae7dcbb057d4f115207f9d8276832a774: I moved code around but kept
>> "optarg" on the cpu_list() call.
>>
>> Reported-by: Jiri Denemark<jdenemar@redhat.com>
>> Signed-off-by: Eduardo Habkost<ehabkost@redhat.com>
>> ---
>>   vl.c |    2 +-
>>   1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/vl.c b/vl.c
>> index 112b0e0..0fccf50 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -3196,7 +3196,7 @@ int main(int argc, char **argv, char **envp)
>>       cpudef_init();
>>
>>       if (cpu_model&&  *cpu_model == '?') {
>> -        list_cpus(stdout,&fprintf, optarg);
>> +        list_cpus(stdout,&fprintf, cpu_model);
>>           exit(0);
>>       }
>>
> Does -cpu ? actually work then? I only see the list of cpus when using
> -cpu ?_ for example.

You're right, it should be cpu_model + 1 from what I recall, i.e. just
the optional argument, not the whole model string.

Andreas
Eduardo Habkost - March 22, 2012, 2:05 p.m.
On Thu, Mar 22, 2012 at 02:46:48PM +0100, Andreas Färber wrote:
> Am 22.03.2012 14:28, schrieb Stefan Berger:
> > On 03/21/2012 08:33 AM, Eduardo Habkost wrote:
> >> Fix stupid copy&paste mistake at commit
> >> ecf40beae7dcbb057d4f115207f9d8276832a774: I moved code around but kept
> >> "optarg" on the cpu_list() call.
> >>
> >> Reported-by: Jiri Denemark<jdenemar@redhat.com>
> >> Signed-off-by: Eduardo Habkost<ehabkost@redhat.com>
> >> ---
> >>   vl.c |    2 +-
> >>   1 files changed, 1 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/vl.c b/vl.c
> >> index 112b0e0..0fccf50 100644
> >> --- a/vl.c
> >> +++ b/vl.c
> >> @@ -3196,7 +3196,7 @@ int main(int argc, char **argv, char **envp)
> >>       cpudef_init();
> >>
> >>       if (cpu_model&&  *cpu_model == '?') {
> >> -        list_cpus(stdout,&fprintf, optarg);
> >> +        list_cpus(stdout,&fprintf, cpu_model);
> >>           exit(0);
> >>       }
> >>
> > Does -cpu ? actually work then? I only see the list of cpus when using
> > -cpu ?_ for example.
> 
> You're right, it should be cpu_model + 1 from what I recall, i.e. just
> the optional argument, not the whole model string.

No, list_cpus() expects the whole argument string, including the leading "?":

void list_cpus(FILE *f, fprintf_function cpu_fprintf, const char *optarg)
{
    /* XXX: implement xxx_cpu_list for targets that still miss it */
#if defined(cpu_list_id)
    cpu_list_id(f, cpu_fprintf, optarg);
#elif defined(cpu_list)
    cpu_list(f, cpu_fprintf); /* deprecated */
#endif
}

[...]

#define cpu_list_id x86_cpu_list

[...]

void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf, const char *optarg)
{
    unsigned char model = !strcmp("?model", optarg);
    unsigned char dump = !strcmp("?dump", optarg);
    unsigned char cpuid = !strcmp("?cpuid", optarg);
[...]
Michael Roth - March 23, 2012, 8:31 p.m.
On Thu, Mar 22, 2012 at 09:28:16AM -0400, Stefan Berger wrote:
> On 03/21/2012 08:33 AM, Eduardo Habkost wrote:
> >Fix stupid copy&paste mistake at commit
> >ecf40beae7dcbb057d4f115207f9d8276832a774: I moved code around but kept
> >"optarg" on the cpu_list() call.
> >
> >Reported-by: Jiri Denemark<jdenemar@redhat.com>
> >Signed-off-by: Eduardo Habkost<ehabkost@redhat.com>
> >---
> >  vl.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> >diff --git a/vl.c b/vl.c
> >index 112b0e0..0fccf50 100644
> >--- a/vl.c
> >+++ b/vl.c
> >@@ -3196,7 +3196,7 @@ int main(int argc, char **argv, char **envp)
> >      cpudef_init();
> >
> >      if (cpu_model&&  *cpu_model == '?') {
> >-        list_cpus(stdout,&fprintf, optarg);
> >+        list_cpus(stdout,&fprintf, cpu_model);
> >          exit(0);
> >      }
> >
> Does -cpu ? actually work then? I only see the list of cpus when
> using -cpu ?_ for example.

I think this is actually just a shell issue:

mdroth@illuin:~/tmp/empty$ ls
mdroth@illuin:~/tmp/empty$ echo ?
?
mdroth@illuin:~/tmp/empty$ touch a
mdroth@illuin:~/tmp/empty$ echo ?
a
mdroth@illuin:~/tmp/empty$ echo ?_
?_

> 
>    Stefan
> 
>
Alon Levy - March 23, 2012, 8:37 p.m.
On Fri, Mar 23, 2012 at 03:31:48PM -0500, Michael Roth wrote:
> On Thu, Mar 22, 2012 at 09:28:16AM -0400, Stefan Berger wrote:
> > On 03/21/2012 08:33 AM, Eduardo Habkost wrote:
> > >Fix stupid copy&paste mistake at commit
> > >ecf40beae7dcbb057d4f115207f9d8276832a774: I moved code around but kept
> > >"optarg" on the cpu_list() call.
> > >
> > >Reported-by: Jiri Denemark<jdenemar@redhat.com>
> > >Signed-off-by: Eduardo Habkost<ehabkost@redhat.com>
> > >---
> > >  vl.c |    2 +-
> > >  1 files changed, 1 insertions(+), 1 deletions(-)
> > >
> > >diff --git a/vl.c b/vl.c
> > >index 112b0e0..0fccf50 100644
> > >--- a/vl.c
> > >+++ b/vl.c
> > >@@ -3196,7 +3196,7 @@ int main(int argc, char **argv, char **envp)
> > >      cpudef_init();
> > >
> > >      if (cpu_model&&  *cpu_model == '?') {
> > >-        list_cpus(stdout,&fprintf, optarg);
> > >+        list_cpus(stdout,&fprintf, cpu_model);
> > >          exit(0);
> > >      }
> > >
> > Does -cpu ? actually work then? I only see the list of cpus when
> > using -cpu ?_ for example.

use "-cpu \?"

> 
> I think this is actually just a shell issue:
> 
> mdroth@illuin:~/tmp/empty$ ls
> mdroth@illuin:~/tmp/empty$ echo ?
> ?
> mdroth@illuin:~/tmp/empty$ touch a
> mdroth@illuin:~/tmp/empty$ echo ?
> a
> mdroth@illuin:~/tmp/empty$ echo ?_
> ?_
> 
> > 
> >    Stefan
> > 
> > 
>

Patch

diff --git a/vl.c b/vl.c
index 112b0e0..0fccf50 100644
--- a/vl.c
+++ b/vl.c
@@ -3196,7 +3196,7 @@  int main(int argc, char **argv, char **envp)
     cpudef_init();
 
     if (cpu_model && *cpu_model == '?') {
-        list_cpus(stdout, &fprintf, optarg);
+        list_cpus(stdout, &fprintf, cpu_model);
         exit(0);
     }