| Message ID | 1491959974-8761-1-git-send-email-luwei.kang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Apr 12, 2017 at 09:19:34AM +0800, Luwei Kang wrote: > User can set max freq to specific cpu by > "xenpm set-scaling-maxfreq <cpuid> <max freq>" > or set max freq to all cpu with default cpuid by > "xenpm set-scaling-maxfreq <max freq>". > > Set max freq with defaule cpuid will cause default > segmentation fault after commit id d4906b5d05. > This patch will fix this issue and add ability > to set max freq with default cpuid. > > Signed-off-by: Luwei Kang <luwei.kang@intel.com> I think this patch basically restores the code to the correct logic before it was broken by d4906b5d05, so: Acked-by: Wei Liu <wei.liu2@citrix.com> CC Julien > --- > tools/misc/xenpm.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/tools/misc/xenpm.c b/tools/misc/xenpm.c > index ded40b9..abe31b5 100644 > --- a/tools/misc/xenpm.c > +++ b/tools/misc/xenpm.c > @@ -99,8 +99,10 @@ static void parse_cpuid_and_int(int argc, char *argv[], > exit(EINVAL); > } > > - parse_cpuid(argv[0], cpuid); > - if ( sscanf(argv[1], "%d", val) != 1 ) > + if ( argc > 1 ) > + parse_cpuid(argv[0], cpuid); > + > + if ( argc == 0 || sscanf(argv[argc > 1], "%d", val) != 1 ) > { > fprintf(stderr, "Invalid %s '%s'\n", what, argv[1]); > exit(EINVAL); > -- > 1.8.3.1 >
Hi Wei, On 12/04/17 15:54, Wei Liu wrote: > On Wed, Apr 12, 2017 at 09:19:34AM +0800, Luwei Kang wrote: >> User can set max freq to specific cpu by >> "xenpm set-scaling-maxfreq <cpuid> <max freq>" >> or set max freq to all cpu with default cpuid by >> "xenpm set-scaling-maxfreq <max freq>". >> >> Set max freq with defaule cpuid will cause > > default > >> segmentation fault after commit id d4906b5d05. >> This patch will fix this issue and add ability >> to set max freq with default cpuid. >> >> Signed-off-by: Luwei Kang <luwei.kang@intel.com> > > I think this patch basically restores the code to the correct logic > before it was broken by d4906b5d05, so: > > Acked-by: Wei Liu <wei.liu2@citrix.com> If I am not mistaken, this patch will re-introduce an error with clang build. From the commit message the sscanf was problematic: xenpm.c:102:23: error: data argument not used by format string [-Werror,-Wformat-extra-args] what, argv[argc > 1]); ^ So we would break clang build but get a segfault fixed :). I have CCed Roger to confirm. Cheers, > > CC Julien > >> --- >> tools/misc/xenpm.c | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/tools/misc/xenpm.c b/tools/misc/xenpm.c >> index ded40b9..abe31b5 100644 >> --- a/tools/misc/xenpm.c >> +++ b/tools/misc/xenpm.c >> @@ -99,8 +99,10 @@ static void parse_cpuid_and_int(int argc, char *argv[], >> exit(EINVAL); >> } >> >> - parse_cpuid(argv[0], cpuid); >> - if ( sscanf(argv[1], "%d", val) != 1 ) >> + if ( argc > 1 ) >> + parse_cpuid(argv[0], cpuid); >> + >> + if ( argc == 0 || sscanf(argv[argc > 1], "%d", val) != 1 ) >> { >> fprintf(stderr, "Invalid %s '%s'\n", what, argv[1]); >> exit(EINVAL); >> -- >> 1.8.3.1 >>
On Thu, Apr 13, 2017 at 10:41:06AM +0100, Julien Grall wrote: > Hi Wei, > > On 12/04/17 15:54, Wei Liu wrote: > > On Wed, Apr 12, 2017 at 09:19:34AM +0800, Luwei Kang wrote: > > > User can set max freq to specific cpu by > > > "xenpm set-scaling-maxfreq <cpuid> <max freq>" > > > or set max freq to all cpu with default cpuid by > > > "xenpm set-scaling-maxfreq <max freq>". > > > > > > Set max freq with defaule cpuid will cause > > > > default > > > > > segmentation fault after commit id d4906b5d05. > > > This patch will fix this issue and add ability > > > to set max freq with default cpuid. > > > > > > Signed-off-by: Luwei Kang <luwei.kang@intel.com> > > > > I think this patch basically restores the code to the correct logic > > before it was broken by d4906b5d05, so: > > > > Acked-by: Wei Liu <wei.liu2@citrix.com> > > If I am not mistaken, this patch will re-introduce an error with clang > build. From the commit message the sscanf was problematic: > > xenpm.c:102:23: error: data argument not used by format string > [-Werror,-Wformat-extra-args] > what, argv[argc > 1]); > ^ That a different thing. The error was due to fprintf(stderr, argc ? "Invalid %s '%s'\n" : "Missing %s\n", what, argv[argc > 1]); When argc was 0 the format string only contained one %s but two data arguments were provided. Wei.
diff --git a/tools/misc/xenpm.c b/tools/misc/xenpm.c index ded40b9..abe31b5 100644 --- a/tools/misc/xenpm.c +++ b/tools/misc/xenpm.c @@ -99,8 +99,10 @@ static void parse_cpuid_and_int(int argc, char *argv[], exit(EINVAL); } - parse_cpuid(argv[0], cpuid); - if ( sscanf(argv[1], "%d", val) != 1 ) + if ( argc > 1 ) + parse_cpuid(argv[0], cpuid); + + if ( argc == 0 || sscanf(argv[argc > 1], "%d", val) != 1 ) { fprintf(stderr, "Invalid %s '%s'\n", what, argv[1]); exit(EINVAL);
User can set max freq to specific cpu by "xenpm set-scaling-maxfreq <cpuid> <max freq>" or set max freq to all cpu with default cpuid by "xenpm set-scaling-maxfreq <max freq>". Set max freq with defaule cpuid will cause segmentation fault after commit id d4906b5d05. This patch will fix this issue and add ability to set max freq with default cpuid. Signed-off-by: Luwei Kang <luwei.kang@intel.com> --- tools/misc/xenpm.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)