* [PATCH] perf: fix incorrect return value for else case @ 2009-12-21 9:22 Wenji Huang 2009-12-21 15:23 ` John Kacur 0 siblings, 1 reply; 5+ messages in thread From: Wenji Huang @ 2009-12-21 9:22 UTC (permalink / raw) To: linux-kernel; +Cc: acme, mingo, Wenji Huang Return original cmd instead of adding prefix. Signed-off-by: Wenji Huang <wenji.huang@oracle.com> --- tools/perf/builtin-help.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/tools/perf/builtin-help.c b/tools/perf/builtin-help.c index 9f810b1..ca77df5 100644 --- a/tools/perf/builtin-help.c +++ b/tools/perf/builtin-help.c @@ -317,7 +317,7 @@ static const char *cmd_to_page(const char *perf_cmd) else if (is_perf_command(perf_cmd)) return prepend("perf-", perf_cmd); else - return prepend("perf-", perf_cmd); + return perf_cmd; } static void setup_man_path(void) -- 1.5.6 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] perf: fix incorrect return value for else case 2009-12-21 9:22 [PATCH] perf: fix incorrect return value for else case Wenji Huang @ 2009-12-21 15:23 ` John Kacur 2009-12-22 1:27 ` Wenji Huang 0 siblings, 1 reply; 5+ messages in thread From: John Kacur @ 2009-12-21 15:23 UTC (permalink / raw) To: Wenji Huang; +Cc: linux-kernel, acme, mingo, Frederic Weisbecker On Mon, Dec 21, 2009 at 10:22 AM, Wenji Huang <wenji.huang@oracle.com> wrote: > Return original cmd instead of adding prefix. > > Signed-off-by: Wenji Huang <wenji.huang@oracle.com> > --- > tools/perf/builtin-help.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/tools/perf/builtin-help.c b/tools/perf/builtin-help.c > index 9f810b1..ca77df5 100644 > --- a/tools/perf/builtin-help.c > +++ b/tools/perf/builtin-help.c > @@ -317,7 +317,7 @@ static const char *cmd_to_page(const char *perf_cmd) > else if (is_perf_command(perf_cmd)) > return prepend("perf-", perf_cmd); > else > - return prepend("perf-", perf_cmd); > + return perf_cmd; > } > > static void setup_man_path(void) > -- > 1.5.6 Sorry - I believe we should NAK this patch. It would turn the following ./perf nonsuchcmd --help No manual entry for perf-nonsuchcmd into ./perf nonsuchcmd --help No manual entry for nonsuchcmd The former is correct, the name of the man page includes the prefix "perf-" NAK (cc-ing Frederic in case he sees it differently) Thank You John Kacur ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] perf: fix incorrect return value for else case 2009-12-21 15:23 ` John Kacur @ 2009-12-22 1:27 ` Wenji Huang 2009-12-30 22:12 ` Frederic Weisbecker 0 siblings, 1 reply; 5+ messages in thread From: Wenji Huang @ 2009-12-22 1:27 UTC (permalink / raw) To: John Kacur; +Cc: linux-kernel, acme, mingo, Frederic Weisbecker John Kacur wrote: > On Mon, Dec 21, 2009 at 10:22 AM, Wenji Huang <wenji.huang@oracle.com> wrote: >> Return original cmd instead of adding prefix. >> >> Signed-off-by: Wenji Huang <wenji.huang@oracle.com> >> --- >> tools/perf/builtin-help.c | 2 +- >> 1 files changed, 1 insertions(+), 1 deletions(-) >> >> diff --git a/tools/perf/builtin-help.c b/tools/perf/builtin-help.c >> index 9f810b1..ca77df5 100644 >> --- a/tools/perf/builtin-help.c >> +++ b/tools/perf/builtin-help.c >> @@ -317,7 +317,7 @@ static const char *cmd_to_page(const char *perf_cmd) >> else if (is_perf_command(perf_cmd)) >> return prepend("perf-", perf_cmd); >> else >> - return prepend("perf-", perf_cmd); >> + return perf_cmd; >> } >> >> static void setup_man_path(void) >> -- >> 1.5.6 > > Sorry - I believe we should NAK this patch. > It would turn the following > > ./perf nonsuchcmd --help > No manual entry for perf-nonsuchcmd > > into > > ./perf nonsuchcmd --help > No manual entry for nonsuchcmd > > The former is correct, the name of the man page includes the prefix "perf-" > > NAK > > (cc-ing Frederic in case he sees it differently) > Thanks. Since we think the former is better, why not make the code compact? Like, diff --git a/tools/perf/builtin-help.c b/tools/perf/builtin-help.c index 9f810b1..65e2691 100644 --- a/tools/perf/builtin-help.c +++ b/tools/perf/builtin-help.c @@ -314,8 +314,6 @@ static const char *cmd_to_page(const char *perf_cmd) return "perf"; else if (!prefixcmp(perf_cmd, "perf")) return perf_cmd; - else if (is_perf_command(perf_cmd)) - return prepend("perf-", perf_cmd); else return prepend("perf-", perf_cmd); } ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] perf: fix incorrect return value for else case 2009-12-22 1:27 ` Wenji Huang @ 2009-12-30 22:12 ` Frederic Weisbecker 2010-01-13 8:45 ` Ingo Molnar 0 siblings, 1 reply; 5+ messages in thread From: Frederic Weisbecker @ 2009-12-30 22:12 UTC (permalink / raw) To: Wenji Huang; +Cc: John Kacur, linux-kernel, acme, mingo On Tue, Dec 22, 2009 at 09:27:12AM +0800, Wenji Huang wrote: > John Kacur wrote: >> On Mon, Dec 21, 2009 at 10:22 AM, Wenji Huang <wenji.huang@oracle.com> wrote: >>> Return original cmd instead of adding prefix. >>> >>> Signed-off-by: Wenji Huang <wenji.huang@oracle.com> >>> --- >>> tools/perf/builtin-help.c | 2 +- >>> 1 files changed, 1 insertions(+), 1 deletions(-) >>> >>> diff --git a/tools/perf/builtin-help.c b/tools/perf/builtin-help.c >>> index 9f810b1..ca77df5 100644 >>> --- a/tools/perf/builtin-help.c >>> +++ b/tools/perf/builtin-help.c >>> @@ -317,7 +317,7 @@ static const char *cmd_to_page(const char *perf_cmd) >>> else if (is_perf_command(perf_cmd)) >>> return prepend("perf-", perf_cmd); >>> else >>> - return prepend("perf-", perf_cmd); >>> + return perf_cmd; >>> } >>> >>> static void setup_man_path(void) >>> -- >>> 1.5.6 >> >> Sorry - I believe we should NAK this patch. >> It would turn the following >> >> ./perf nonsuchcmd --help >> No manual entry for perf-nonsuchcmd >> >> into >> >> ./perf nonsuchcmd --help >> No manual entry for nonsuchcmd >> >> The former is correct, the name of the man page includes the prefix "perf-" >> >> NAK >> >> (cc-ing Frederic in case he sees it differently) I personally don't mind. Having either ./perf nonsuchcmd --help No manual entry for perf-nonsuchcmd or ./perf nonsuchcmd --help No manual entry for nonsuchcmd both make sense for the user. >> > Thanks. Since we think the former is better, why not make > the code compact? Like, > > diff --git a/tools/perf/builtin-help.c b/tools/perf/builtin-help.c > index 9f810b1..65e2691 100644 > --- a/tools/perf/builtin-help.c > +++ b/tools/perf/builtin-help.c > @@ -314,8 +314,6 @@ static const char *cmd_to_page(const char *perf_cmd) > return "perf"; > else if (!prefixcmp(perf_cmd, "perf")) > return perf_cmd; > - else if (is_perf_command(perf_cmd)) > - return prepend("perf-", perf_cmd); > else > return prepend("perf-", perf_cmd); > } Agreed! Thanks. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] perf: fix incorrect return value for else case 2009-12-30 22:12 ` Frederic Weisbecker @ 2010-01-13 8:45 ` Ingo Molnar 0 siblings, 0 replies; 5+ messages in thread From: Ingo Molnar @ 2010-01-13 8:45 UTC (permalink / raw) To: Frederic Weisbecker; +Cc: Wenji Huang, John Kacur, linux-kernel, acme * Frederic Weisbecker <fweisbec@gmail.com> wrote: > On Tue, Dec 22, 2009 at 09:27:12AM +0800, Wenji Huang wrote: > > John Kacur wrote: > >> On Mon, Dec 21, 2009 at 10:22 AM, Wenji Huang <wenji.huang@oracle.com> wrote: > >>> Return original cmd instead of adding prefix. > >>> > >>> Signed-off-by: Wenji Huang <wenji.huang@oracle.com> > >>> --- > >>> tools/perf/builtin-help.c | 2 +- > >>> 1 files changed, 1 insertions(+), 1 deletions(-) > >>> > >>> diff --git a/tools/perf/builtin-help.c b/tools/perf/builtin-help.c > >>> index 9f810b1..ca77df5 100644 > >>> --- a/tools/perf/builtin-help.c > >>> +++ b/tools/perf/builtin-help.c > >>> @@ -317,7 +317,7 @@ static const char *cmd_to_page(const char *perf_cmd) > >>> else if (is_perf_command(perf_cmd)) > >>> return prepend("perf-", perf_cmd); > >>> else > >>> - return prepend("perf-", perf_cmd); > >>> + return perf_cmd; > >>> } > >>> > >>> static void setup_man_path(void) > >>> -- > >>> 1.5.6 > >> > >> Sorry - I believe we should NAK this patch. > >> It would turn the following > >> > >> ./perf nonsuchcmd --help > >> No manual entry for perf-nonsuchcmd > >> > >> into > >> > >> ./perf nonsuchcmd --help > >> No manual entry for nonsuchcmd > >> > >> The former is correct, the name of the man page includes the prefix "perf-" > >> > >> NAK > >> > >> (cc-ing Frederic in case he sees it differently) > > > I personally don't mind. > Having either > > ./perf nonsuchcmd --help > No manual entry for perf-nonsuchcmd > or > > ./perf nonsuchcmd --help > No manual entry for nonsuchcmd > > both make sense for the user. > > > >> > > Thanks. Since we think the former is better, why not make > > the code compact? Like, > > > > diff --git a/tools/perf/builtin-help.c b/tools/perf/builtin-help.c > > index 9f810b1..65e2691 100644 > > --- a/tools/perf/builtin-help.c > > +++ b/tools/perf/builtin-help.c > > @@ -314,8 +314,6 @@ static const char *cmd_to_page(const char *perf_cmd) > > return "perf"; > > else if (!prefixcmp(perf_cmd, "perf")) > > return perf_cmd; > > - else if (is_perf_command(perf_cmd)) > > - return prepend("perf-", perf_cmd); > > else > > return prepend("perf-", perf_cmd); > > } > > > Agreed! > Thanks. if this looks good to everyone please re-send the patch with a changelog and with acks added. Thanks, Ingo ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-01-13 8:47 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-12-21 9:22 [PATCH] perf: fix incorrect return value for else case Wenji Huang 2009-12-21 15:23 ` John Kacur 2009-12-22 1:27 ` Wenji Huang 2009-12-30 22:12 ` Frederic Weisbecker 2010-01-13 8:45 ` Ingo Molnar
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox