[PATCH] : eliminate code duplication in kernel/tracepoint.c

[PATCH] : eliminate code duplication in kernel/tracepoint.c

[Anirban Sinha]

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

Re: [PATCH] : eliminate code duplication in kernel/tracepoint.c

[Li Zefan]

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
> 

RE: [PATCH] : eliminate code duplication in kernel/tracepoint.c

[Anirban Sinha]

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

Re: [PATCH] : eliminate code duplication in kernel/tracepoint.c

[Li Zefan]

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

Re: [PATCH] : eliminate code duplication in kernel/tracepoint.c

[Ingo Molnar]

* 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

RE: [PATCH] : eliminate code duplication in kernel/tracepoint.c

[Anirban Sinha]

>> 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

[tip:tracing/core] tracing: Eliminate code duplication in kernel/tracepoint.c

[tip-bot for Anirban Sinha]

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