All of lore.kernel.org
 help / color / mirror / Atom feed
From: Wenji Huang <wenji.huang@oracle.com>
To: John Kacur <jkacur@redhat.com>
Cc: linux-kernel@vger.kernel.org, acme@redhat.com, mingo@elte.hu,
	Frederic Weisbecker <fweisbec@gmail.com>
Subject: Re: [PATCH] perf: fix incorrect return value for else case
Date: Tue, 22 Dec 2009 09:27:12 +0800	[thread overview]
Message-ID: <4B302070.5000905@oracle.com> (raw)
In-Reply-To: <520f0cf10912210723j1ac91183n3d7970b96b95ef23@mail.gmail.com>

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);
  }

  reply	other threads:[~2009-12-22  1:32 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2009-12-30 22:12     ` Frederic Weisbecker
2010-01-13  8:45       ` Ingo Molnar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4B302070.5000905@oracle.com \
    --to=wenji.huang@oracle.com \
    --cc=acme@redhat.com \
    --cc=fweisbec@gmail.com \
    --cc=jkacur@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.