Hello,
here is a small patch to the perf tool to use strlen & macros
instead of manually counting string lengths.
I didn't find mention of any maintainer for the tool...
Please advise if I should submit this in another way
--
Vincent Legoll
* Vincent Legoll <[email protected]> wrote:
> Hello,
>
> here is a small patch to the perf tool to use strlen & macros
> instead of manually counting string lengths.
>
> I didn't find mention of any maintainer for the tool...
It's maintained by:
PERFORMANCE EVENTS SUBSYSTEM
M: Peter Zijlstra <[email protected]>
M: Paul Mackerras <[email protected]>
M: Ingo Molnar <[email protected]>
S: Supported
We should add 'F:' file patterns to that entry i guess ...
Something like:
F: kernel/perf_event.c
F: include/linux/perf_event.h
F: arch/*/*/kernel/perf_event.c
F: arch/*/include/asm/perf_event.h
F: tools/perf/
Would do the trick. Mind sending a patch for that too?
> Please advise if I should submit this in another way
Submitting it to lkml was fine enough - please Cc: it to us in the
future to make sure we dont miss it.
Applied, thanks for the patch!
Ingo
Commit-ID: cfed95a693e1ea5d08b9c9019bc30e448437ee2f
Gitweb: http://git.kernel.org/tip/cfed95a693e1ea5d08b9c9019bc30e448437ee2f
Author: Vincent Legoll <[email protected]>
AuthorDate: Tue, 13 Oct 2009 10:18:16 +0200
Committer: Ingo Molnar <[email protected]>
CommitDate: Tue, 13 Oct 2009 11:55:31 +0200
perf tools: Do not manually count string lengths
Use strlen & macros instead of manually counting string lengths as
this is error prone and may lend to bugs.
Signed-off-by: Vincent Legoll <[email protected]>
Cc: Linus Torvalds <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
tools/perf/perf.c | 16 ++++++++--------
tools/perf/util/cache.h | 5 +++++
2 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index 19fc7fe..624e62d 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -89,8 +89,8 @@ static int handle_options(const char*** argv, int* argc, int* envchanged)
/*
* Check remaining flags.
*/
- if (!prefixcmp(cmd, "--exec-path")) {
- cmd += 11;
+ if (!prefixcmp(cmd, CMD_EXEC_PATH)) {
+ cmd += strlen(CMD_EXEC_PATH);
if (*cmd == '=')
perf_set_argv_exec_path(cmd + 1);
else {
@@ -117,8 +117,8 @@ static int handle_options(const char*** argv, int* argc, int* envchanged)
(*argv)++;
(*argc)--;
handled++;
- } else if (!prefixcmp(cmd, "--perf-dir=")) {
- setenv(PERF_DIR_ENVIRONMENT, cmd + 10, 1);
+ } else if (!prefixcmp(cmd, CMD_PERF_DIR)) {
+ setenv(PERF_DIR_ENVIRONMENT, cmd + strlen(CMD_PERF_DIR), 1);
if (envchanged)
*envchanged = 1;
} else if (!strcmp(cmd, "--work-tree")) {
@@ -131,8 +131,8 @@ static int handle_options(const char*** argv, int* argc, int* envchanged)
*envchanged = 1;
(*argv)++;
(*argc)--;
- } else if (!prefixcmp(cmd, "--work-tree=")) {
- setenv(PERF_WORK_TREE_ENVIRONMENT, cmd + 12, 1);
+ } else if (!prefixcmp(cmd, CMD_WORK_TREE)) {
+ setenv(PERF_WORK_TREE_ENVIRONMENT, cmd + strlen(CMD_WORK_TREE), 1);
if (envchanged)
*envchanged = 1;
} else if (!strcmp(cmd, "--debugfs-dir")) {
@@ -146,8 +146,8 @@ static int handle_options(const char*** argv, int* argc, int* envchanged)
*envchanged = 1;
(*argv)++;
(*argc)--;
- } else if (!prefixcmp(cmd, "--debugfs-dir=")) {
- strncpy(debugfs_mntpt, cmd + 14, MAXPATHLEN);
+ } else if (!prefixcmp(cmd, CMD_DEBUGFS_DIR)) {
+ strncpy(debugfs_mntpt, cmd + strlen(CMD_DEBUGFS_DIR), MAXPATHLEN);
debugfs_mntpt[MAXPATHLEN - 1] = '\0';
if (envchanged)
*envchanged = 1;
diff --git a/tools/perf/util/cache.h b/tools/perf/util/cache.h
index f26172c..918eb37 100644
--- a/tools/perf/util/cache.h
+++ b/tools/perf/util/cache.h
@@ -5,6 +5,11 @@
#include "strbuf.h"
#include "../perf.h"
+#define CMD_EXEC_PATH "--exec-path"
+#define CMD_PERF_DIR "--perf-dir="
+#define CMD_WORK_TREE "--work-tree="
+#define CMD_DEBUGFS_DIR "--debugfs-dir="
+
#define PERF_DIR_ENVIRONMENT "PERF_DIR"
#define PERF_WORK_TREE_ENVIRONMENT "PERF_WORK_TREE"
#define DEFAULT_PERF_DIR_ENVIRONMENT ".perf"
On Tue, Oct 13, 2009 at 12:01 PM, Ingo Molnar <[email protected]> wrote:
> We should add 'F:' file patterns to that entry i guess ...
>
> Something like:
>
> F: ? ? ?kernel/perf_event.c
> F: ? ? ?include/linux/perf_event.h
> F: ? ? ?arch/*/*/kernel/perf_event.c
> F: ? ? ?arch/*/include/asm/perf_event.h
> F: ? ? ?tools/perf/
>
> Would do the trick. Mind sending a patch for that too?
Yep, I'll do, but a quick find in the tree may reveal other
suspects that could be in the same league:
$ find . -name .git -prune -false -o -name drivers -prune \
-false -o -name '*perf*' | grep -v perfmon | grep -v gperf
arch/*/kernel/perf_event.c
arch/frv/lib/perf_event.c
arch/x86/kernel/cpu/perf_event.c
or maybe not:
arch/powerpc/kernel/perf_callchain.c
arch/parisc/kernel/perf_asm.S
arch/parisc/kernel/perf_images.h
arch/parisc/kernel/perf.c
arch/parisc/include/asm/perf.h
Was your list really comprehensive ?
--
Vincent Legoll
* Vincent Legoll <[email protected]> wrote:
> On Tue, Oct 13, 2009 at 12:01 PM, Ingo Molnar <[email protected]> wrote:
> > We should add 'F:' file patterns to that entry i guess ...
> >
> > Something like:
> >
> > F: ? ? ?kernel/perf_event.c
> > F: ? ? ?include/linux/perf_event.h
> > F: ? ? ?arch/*/*/kernel/perf_event.c
> > F: ? ? ?arch/*/include/asm/perf_event.h
> > F: ? ? ?tools/perf/
> >
> > Would do the trick. Mind sending a patch for that too?
>
> Yep, I'll do, but a quick find in the tree may reveal other
> suspects that could be in the same league:
>
> $ find . -name .git -prune -false -o -name drivers -prune \
> -false -o -name '*perf*' | grep -v perfmon | grep -v gperf
>
> arch/*/kernel/perf_event.c
>
> arch/frv/lib/perf_event.c
> arch/x86/kernel/cpu/perf_event.c
>
> or maybe not:
>
> arch/powerpc/kernel/perf_callchain.c
> arch/parisc/kernel/perf_asm.S
> arch/parisc/kernel/perf_images.h
> arch/parisc/kernel/perf.c
> arch/parisc/include/asm/perf.h
>
> Was your list really comprehensive ?
ah, yes - perf_callchain.c too indeed. The pattern does not have to be
100% full coverage.
Ingo
On Tue, Oct 13, 2009 at 2:27 PM, Ingo Molnar <[email protected]> wrote:
>> Was your list really comprehensive ?
>
> ah, yes - perf_callchain.c too indeed. The pattern does not have to be
> 100% full coverage.
How looks the attached ?
--
Vincent Legoll
Hi Vincent,
On Tue, Oct 13, 2009 at 2:27 PM, Ingo Molnar <[email protected]> wrote:
>>> Was your list really comprehensive ?
>>
>> ah, yes - perf_callchain.c too indeed. The pattern does not have to be
>> 100% full coverage.
On Tue, Oct 13, 2009 at 3:48 PM, Vincent Legoll
<[email protected]> wrote:
> How looks the attached ?
Can you please send patches as plain-text in the future? Attachments
are pain to review on some email clients (such as mine).
Pekka
Commit-ID: a003236c32706f3c1f74d4e3b98c58cf0d9a9d8f
Gitweb: http://git.kernel.org/tip/a003236c32706f3c1f74d4e3b98c58cf0d9a9d8f
Author: Vincent Legoll <[email protected]>
AuthorDate: Tue, 13 Oct 2009 14:48:14 +0200
Committer: Ingo Molnar <[email protected]>
CommitDate: Tue, 13 Oct 2009 15:28:52 +0200
perf events: Update MAINTAINERS entry file patterns
Add file patterns that match relevant files for this subsystem.
Signed-off-by: Vincent Legoll <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: [email protected]
Cc: [email protected]
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
MAINTAINERS | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 69e31aa..cf69091 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4071,6 +4071,13 @@ M: Peter Zijlstra <[email protected]>
M: Paul Mackerras <[email protected]>
M: Ingo Molnar <[email protected]>
S: Supported
+F: kernel/perf_event.c
+F: include/linux/perf_event.h
+F: arch/*/*/kernel/perf_event.c
+F: arch/*/include/asm/perf_event.h
+F: arch/*/lib/perf_event.c
+F: arch/*/kernel/perf_callchain.c
+F: tools/perf/
PERSONALITY HANDLING
M: Christoph Hellwig <[email protected]>