* [PATCH] : eliminate code duplication in kernel/tracepoint.c
@ 2009-08-24 23:16 Anirban Sinha
2009-08-25 1:20 ` Li Zefan
0 siblings, 1 reply; 7+ messages in thread
From: Anirban Sinha @ 2009-08-24 23:16 UTC (permalink / raw)
To: Ingo Molnar; +Cc: linux-kernel, Oleg Nesterov
Hi Ingo:
I was going through the kernel tracepoints work. It's really impressive.
Congratulations!
The following cleanup popped up immediately. May be you guys have
noticed it already.
Cheers,
Ani
>From 2068b87b9635a66e5e86b8ff4182d96a6e969584 Mon Sep 17 00:00:00 2001
From: Anirban Sinha <asinha@zeugmasystems.com>
Date: Mon, 24 Aug 2009 15:52:20 -0700
Subject: Cleanup: eliminate code duplication in kernel/tracepoint.c
Signed-off-by: Anirban Sinha <asinha@zeugmasystems.com>
---
kernel/tracepoint.c | 3 ---
1 files changed, 0 insertions(+), 3 deletions(-)
diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
index 1ef5d3a..4840ab6 100644
--- a/kernel/tracepoint.c
+++ b/kernel/tracepoint.c
@@ -554,9 +554,6 @@ int tracepoint_module_notify(struct notifier_block
*self,
switch (val) {
case MODULE_STATE_COMING:
- tracepoint_update_probe_range(mod->tracepoints,
- mod->tracepoints + mod->num_tracepoints);
- break;
case MODULE_STATE_GOING:
tracepoint_update_probe_range(mod->tracepoints,
mod->tracepoints + mod->num_tracepoints);
--
1.6.4
>From 5e09b156be90b936db9c1977abcc04b5cbb4bd4a Mon Sep 17 00:00:00 2001
From: Anirban Sinha <asinha@zeugmasystems.com>
Date: Mon, 24 Aug 2009 15:55:12 -0700
Subject: cleanup: remove redundant break statement.
Signed-off-by: Anirban Sinha <asinha@zeugmasystems.com>
---
kernel/tracepoint.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
index 4840ab6..35eed9c 100644
--- a/kernel/tracepoint.c
+++ b/kernel/tracepoint.c
@@ -557,7 +557,6 @@ int tracepoint_module_notify(struct notifier_block
*self,
case MODULE_STATE_GOING:
tracepoint_update_probe_range(mod->tracepoints,
mod->tracepoints + mod->num_tracepoints);
- break;
}
return 0;
}
--
1.6.4
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] : eliminate code duplication in kernel/tracepoint.c
2009-08-24 23:16 [PATCH] : eliminate code duplication in kernel/tracepoint.c Anirban Sinha
@ 2009-08-25 1:20 ` Li Zefan
2009-08-25 3:57 ` Anirban Sinha
0 siblings, 1 reply; 7+ messages in thread
From: Li Zefan @ 2009-08-25 1:20 UTC (permalink / raw)
To: Anirban Sinha; +Cc: Ingo Molnar, linux-kernel, Oleg Nesterov
Anirban Sinha wrote:
> Hi Ingo:
>
> I was going through the kernel tracepoints work. It's really impressive.
> Congratulations!
>
> The following cleanup popped up immediately. May be you guys have
> noticed it already.
>
Maybe, but no one else sent this patch. ;)
> Cheers,
>
> Ani
>
>
>>From 2068b87b9635a66e5e86b8ff4182d96a6e969584 Mon Sep 17 00:00:00 2001
> From: Anirban Sinha <asinha@zeugmasystems.com>
> Date: Mon, 24 Aug 2009 15:52:20 -0700
> Subject: Cleanup: eliminate code duplication in kernel/tracepoint.c
>
>
> Signed-off-by: Anirban Sinha <asinha@zeugmasystems.com>
Reviewed-by: Li Zefan <lizf@cn.fujitsu.com>
> ---
> kernel/tracepoint.c | 3 ---
> 1 files changed, 0 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
> index 1ef5d3a..4840ab6 100644
> --- a/kernel/tracepoint.c
> +++ b/kernel/tracepoint.c
> @@ -554,9 +554,6 @@ int tracepoint_module_notify(struct notifier_block
> *self,
>
> switch (val) {
> case MODULE_STATE_COMING:
> - tracepoint_update_probe_range(mod->tracepoints,
> - mod->tracepoints + mod->num_tracepoints);
> - break;
> case MODULE_STATE_GOING:
> tracepoint_update_probe_range(mod->tracepoints,
> mod->tracepoints + mod->num_tracepoints);
> --
> 1.6.4
>
>
>>From 5e09b156be90b936db9c1977abcc04b5cbb4bd4a Mon Sep 17 00:00:00 2001
> From: Anirban Sinha <asinha@zeugmasystems.com>
> Date: Mon, 24 Aug 2009 15:55:12 -0700
> Subject: cleanup: remove redundant break statement.
>
Oh, you should send 2 mails, one mail per patch.
But removing the break won't gain us anything. Actually It's
better to reserve the break.
>
> Signed-off-by: Anirban Sinha <asinha@zeugmasystems.com>
> ---
> kernel/tracepoint.c | 1 -
> 1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
> index 4840ab6..35eed9c 100644
> --- a/kernel/tracepoint.c
> +++ b/kernel/tracepoint.c
> @@ -557,7 +557,6 @@ int tracepoint_module_notify(struct notifier_block
> *self,
> case MODULE_STATE_GOING:
> tracepoint_update_probe_range(mod->tracepoints,
> mod->tracepoints + mod->num_tracepoints);
> - break;
> }
> return 0;
> }
> --
> 1.6.4
>
^ permalink raw reply [flat|nested] 7+ messages in thread* RE: [PATCH] : eliminate code duplication in kernel/tracepoint.c
2009-08-25 1:20 ` Li Zefan
@ 2009-08-25 3:57 ` Anirban Sinha
2009-08-25 5:26 ` Li Zefan
0 siblings, 1 reply; 7+ messages in thread
From: Anirban Sinha @ 2009-08-25 3:57 UTC (permalink / raw)
To: Li Zefan; +Cc: Ingo Molnar, linux-kernel, Oleg Nesterov
OK, here's the modified patch:
>From 52cea59801ac5b772c49ae995f4df1940a0d88fa Mon Sep 17 00:00:00 2001
From: Anirban Sinha <asinha@zeugmasystems.com>
Date: Mon, 24 Aug 2009 20:52:35 -0700
Subject: cleanup: eliminate code duplication in kernel/tracepoint.c
Signed-off-by: Anirban Sinha <asinha@zeugmasystems.com>
---
kernel/tracepoint.c | 4 ----
1 files changed, 0 insertions(+), 4 deletions(-)
diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
index 1ef5d3a..35eed9c 100644
--- a/kernel/tracepoint.c
+++ b/kernel/tracepoint.c
@@ -554,13 +554,9 @@ int tracepoint_module_notify(struct notifier_block
*self,
switch (val) {
case MODULE_STATE_COMING:
- tracepoint_update_probe_range(mod->tracepoints,
- mod->tracepoints + mod->num_tracepoints);
- break;
case MODULE_STATE_GOING:
tracepoint_update_probe_range(mod->tracepoints,
mod->tracepoints + mod->num_tracepoints);
- break;
}
return 0;
}
--
1.6.4
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] : eliminate code duplication in kernel/tracepoint.c
2009-08-25 3:57 ` Anirban Sinha
@ 2009-08-25 5:26 ` Li Zefan
2009-08-25 6:39 ` Ingo Molnar
0 siblings, 1 reply; 7+ messages in thread
From: Li Zefan @ 2009-08-25 5:26 UTC (permalink / raw)
To: Anirban Sinha; +Cc: Ingo Molnar, linux-kernel, Oleg Nesterov
11:57, Anirban Sinha wrote:
> OK, here's the modified patch:
>
>>From 52cea59801ac5b772c49ae995f4df1940a0d88fa Mon Sep 17 00:00:00 2001
> From: Anirban Sinha <asinha@zeugmasystems.com>
> Date: Mon, 24 Aug 2009 20:52:35 -0700
> Subject: cleanup: eliminate code duplication in kernel/tracepoint.c
>
>
> Signed-off-by: Anirban Sinha <asinha@zeugmasystems.com>
> ---
> kernel/tracepoint.c | 4 ----
> 1 files changed, 0 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
> index 1ef5d3a..35eed9c 100644
> --- a/kernel/tracepoint.c
> +++ b/kernel/tracepoint.c
> @@ -554,13 +554,9 @@ int tracepoint_module_notify(struct notifier_block
> *self,
>
> switch (val) {
> case MODULE_STATE_COMING:
> - tracepoint_update_probe_range(mod->tracepoints,
> - mod->tracepoints + mod->num_tracepoints);
> - break;
> case MODULE_STATE_GOING:
> tracepoint_update_probe_range(mod->tracepoints,
> mod->tracepoints + mod->num_tracepoints);
> - break;
I still think it's better not to remove this break. every "case"
should have a break or a return, expect for the falling throught
cases.
> }
> return 0;
> }
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] : eliminate code duplication in kernel/tracepoint.c
2009-08-25 5:26 ` Li Zefan
@ 2009-08-25 6:39 ` Ingo Molnar
2009-08-25 14:00 ` Anirban Sinha
0 siblings, 1 reply; 7+ messages in thread
From: Ingo Molnar @ 2009-08-25 6:39 UTC (permalink / raw)
To: Li Zefan; +Cc: Anirban Sinha, linux-kernel, Oleg Nesterov
* Li Zefan <lizf@cn.fujitsu.com> wrote:
> 11:57, Anirban Sinha wrote:
> > OK, here's the modified patch:
> >
> >>From 52cea59801ac5b772c49ae995f4df1940a0d88fa Mon Sep 17 00:00:00 2001
> > From: Anirban Sinha <asinha@zeugmasystems.com>
> > Date: Mon, 24 Aug 2009 20:52:35 -0700
> > Subject: cleanup: eliminate code duplication in kernel/tracepoint.c
> >
> >
> > Signed-off-by: Anirban Sinha <asinha@zeugmasystems.com>
> > ---
> > kernel/tracepoint.c | 4 ----
> > 1 files changed, 0 insertions(+), 4 deletions(-)
> >
> > diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
> > index 1ef5d3a..35eed9c 100644
> > --- a/kernel/tracepoint.c
> > +++ b/kernel/tracepoint.c
> > @@ -554,13 +554,9 @@ int tracepoint_module_notify(struct notifier_block
> > *self,
> >
> > switch (val) {
> > case MODULE_STATE_COMING:
> > - tracepoint_update_probe_range(mod->tracepoints,
> > - mod->tracepoints + mod->num_tracepoints);
> > - break;
> > case MODULE_STATE_GOING:
> > tracepoint_update_probe_range(mod->tracepoints,
> > mod->tracepoints + mod->num_tracepoints);
> > - break;
>
> I still think it's better not to remove this break. every "case"
> should have a break or a return, expect for the falling throught
> cases.
Correct. That might seem like a superfluous statement, but when we
add new cases it stays robust while with the missing break one can
create an unintended fall-through codepath.
Ingo
^ permalink raw reply [flat|nested] 7+ messages in thread* RE: [PATCH] : eliminate code duplication in kernel/tracepoint.c
2009-08-25 6:39 ` Ingo Molnar
@ 2009-08-25 14:00 ` Anirban Sinha
2009-08-25 14:18 ` [tip:tracing/core] tracing: Eliminate " tip-bot for Anirban Sinha
0 siblings, 1 reply; 7+ messages in thread
From: Anirban Sinha @ 2009-08-25 14:00 UTC (permalink / raw)
To: Ingo Molnar, Li Zefan; +Cc: linux-kernel, Oleg Nesterov
>> I still think it's better not to remove this break. every "case"
>> should have a break or a return, expect for the falling throught
>> cases.
>
>Correct. That might seem like a superfluous statement, but when we
>add new cases it stays robust while with the missing break one can
>create an unintended fall-through codepath.
>
> Ingo
Thanks a lot Li and Ingo for all the feedback. Final patch goes below:
>From f9826874841a5b15b553cfb6540d1ecfa6b55531 Mon Sep 17 00:00:00 2001
From: Anirban Sinha <asinha@zeugmasystems.com>
Date: Tue, 25 Aug 2009 06:50:54 -0700
Subject: cleanup: eliminate code duplication in kernel/tracepoint.c
Signed-off-by: Anirban Sinha <asinha@zeugmasystems.com>
Reviewed-by: Li Zefan <lizf@cn.fujitsu.com>
---
kernel/tracepoint.c | 3 ---
1 files changed, 0 insertions(+), 3 deletions(-)
diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
index 1ef5d3a..4840ab6 100644
--- a/kernel/tracepoint.c
+++ b/kernel/tracepoint.c
@@ -554,9 +554,6 @@ int tracepoint_module_notify(struct notifier_block
*self,
switch (val) {
case MODULE_STATE_COMING:
- tracepoint_update_probe_range(mod->tracepoints,
- mod->tracepoints + mod->num_tracepoints);
- break;
case MODULE_STATE_GOING:
tracepoint_update_probe_range(mod->tracepoints,
mod->tracepoints + mod->num_tracepoints);
--
1.6.4
^ permalink raw reply related [flat|nested] 7+ messages in thread* [tip:tracing/core] tracing: Eliminate code duplication in kernel/tracepoint.c
2009-08-25 14:00 ` Anirban Sinha
@ 2009-08-25 14:18 ` tip-bot for Anirban Sinha
0 siblings, 0 replies; 7+ messages in thread
From: tip-bot for Anirban Sinha @ 2009-08-25 14:18 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, hpa, mingo, lizf, asinha, ASinha, oleg, tglx, mingo
Commit-ID: d88cb582325830698de5071fa8b8c9e933dbbcad
Gitweb: http://git.kernel.org/tip/d88cb582325830698de5071fa8b8c9e933dbbcad
Author: Anirban Sinha <ASinha@zeugmasystems.com>
AuthorDate: Tue, 25 Aug 2009 07:00:02 -0700
Committer: Ingo Molnar <mingo@elte.hu>
CommitDate: Tue, 25 Aug 2009 16:15:12 +0200
tracing: Eliminate code duplication in kernel/tracepoint.c
Signed-off-by: Anirban Sinha <asinha@zeugmasystems.com>
Reviewed-by: Li Zefan <lizf@cn.fujitsu.com>
Cc: "Oleg Nesterov" <oleg@tv-sign.ru>
LKML-Reference: <DDFD17CC94A9BD49A82147DDF7D545C501EA9047@exchange.ZeugmaSystems.local>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
kernel/tracepoint.c | 3 ---
1 files changed, 0 insertions(+), 3 deletions(-)
diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
index 35dd27a..06f165a 100644
--- a/kernel/tracepoint.c
+++ b/kernel/tracepoint.c
@@ -555,9 +555,6 @@ int tracepoint_module_notify(struct notifier_block *self,
switch (val) {
case MODULE_STATE_COMING:
- tracepoint_update_probe_range(mod->tracepoints,
- mod->tracepoints + mod->num_tracepoints);
- break;
case MODULE_STATE_GOING:
tracepoint_update_probe_range(mod->tracepoints,
mod->tracepoints + mod->num_tracepoints);
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2009-08-25 14:19 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-08-24 23:16 [PATCH] : eliminate code duplication in kernel/tracepoint.c Anirban Sinha
2009-08-25 1:20 ` Li Zefan
2009-08-25 3:57 ` Anirban Sinha
2009-08-25 5:26 ` Li Zefan
2009-08-25 6:39 ` Ingo Molnar
2009-08-25 14:00 ` Anirban Sinha
2009-08-25 14:18 ` [tip:tracing/core] tracing: Eliminate " tip-bot for Anirban Sinha
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox