Messages in this thread |  | | | Date | Wed, 13 Jan 2010 09:45:40 +0100 | | From | Ingo Molnar <> | | Subject | Re: [PATCH] perf: fix incorrect return value for else case |
| |
* 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
|  |