[PATCH perf/core] perf tools: Add missing break for PERF_RECORD_ITRACE_START

[PATCH perf/core] perf tools: Add missing break for PERF_RECORD_ITRACE_START

[Jiri Olsa]

Missing switch break since introduction of new event:
  c4937a91ea56 perf tools: handle PERF_RECORD_LOST_SAMPLES

Cc: Kan Liang <kan.liang@intel.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Link: http://lkml.kernel.org/n/tip-e3gbrp2561x2s9tkqvf2wh9n@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/machine.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 4744673aff1b..a08e38339cac 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -1448,7 +1448,7 @@ int machine__process_event(struct machine *machine, union perf_event *event,
 	case PERF_RECORD_AUX:
 		ret = machine__process_aux_event(machine, event); break;
 	case PERF_RECORD_ITRACE_START:
-		ret = machine__process_itrace_start_event(machine, event);
+		ret = machine__process_itrace_start_event(machine, event); break;
 	case PERF_RECORD_LOST_SAMPLES:
 		ret = machine__process_lost_samples_event(machine, event, sample); break;
 		break;
-- 
1.9.3

Re: [PATCH perf/core] perf tools: Add missing break for PERF_RECORD_ITRACE_START

[Adrian Hunter]

On 29/06/15 14:12, Jiri Olsa wrote:
> Missing switch break since introduction of new event:
>   c4937a91ea56 perf tools: handle PERF_RECORD_LOST_SAMPLES
> 
> Cc: Kan Liang <kan.liang@intel.com>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Link: http://lkml.kernel.org/n/tip-e3gbrp2561x2s9tkqvf2wh9n@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/perf/util/machine.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index 4744673aff1b..a08e38339cac 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -1448,7 +1448,7 @@ int machine__process_event(struct machine *machine, union perf_event *event,
>  	case PERF_RECORD_AUX:
>  		ret = machine__process_aux_event(machine, event); break;
>  	case PERF_RECORD_ITRACE_START:
> -		ret = machine__process_itrace_start_event(machine, event);
> +		ret = machine__process_itrace_start_event(machine, event); break;
>  	case PERF_RECORD_LOST_SAMPLES:
>  		ret = machine__process_lost_samples_event(machine, event, sample); break;
>  		break;

But now you have break; break;

Isn't putting 'break' on the end of the line making things harder to read?

Re: [PATCH perf/core] perf tools: Add missing break for PERF_RECORD_ITRACE_START

[Joe Perches]

On Mon, 2015-06-29 at 13:12 +0200, Jiri Olsa wrote:
> Missing switch break since introduction of new event:
>   c4937a91ea56 perf tools: handle PERF_RECORD_LOST_SAMPLES
[]
> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
[]
> @@ -1448,7 +1448,7 @@ int machine__process_event(struct machine *machine, union perf_event *event,
>  	case PERF_RECORD_AUX:
>  		ret = machine__process_aux_event(machine, event); break;
>  	case PERF_RECORD_ITRACE_START:
> -		ret = machine__process_itrace_start_event(machine, event);
> +		ret = machine__process_itrace_start_event(machine, event); break;
>  	case PERF_RECORD_LOST_SAMPLES:
>  		ret = machine__process_lost_samples_event(machine, event, sample); break;
>  		break;

trivia: There's a duplicate/unreachable break;
        on the PERF_RECORD_LOST_SAMPLES case

[PATCHv2 perf/core] perf tools: Add missing break for PERF_RECORD_ITRACE_START

[Jiri Olsa]

On Mon, Jun 29, 2015 at 02:17:18PM +0300, Adrian Hunter wrote:
> On 29/06/15 14:12, Jiri Olsa wrote:
> > Missing switch break since introduction of new event:
> >   c4937a91ea56 perf tools: handle PERF_RECORD_LOST_SAMPLES
> > 
> > Cc: Kan Liang <kan.liang@intel.com>
> > Cc: Adrian Hunter <adrian.hunter@intel.com>
> > Link: http://lkml.kernel.org/n/tip-e3gbrp2561x2s9tkqvf2wh9n@git.kernel.org
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  tools/perf/util/machine.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> > index 4744673aff1b..a08e38339cac 100644
> > --- a/tools/perf/util/machine.c
> > +++ b/tools/perf/util/machine.c
> > @@ -1448,7 +1448,7 @@ int machine__process_event(struct machine *machine, union perf_event *event,
> >  	case PERF_RECORD_AUX:
> >  		ret = machine__process_aux_event(machine, event); break;
> >  	case PERF_RECORD_ITRACE_START:
> > -		ret = machine__process_itrace_start_event(machine, event);
> > +		ret = machine__process_itrace_start_event(machine, event); break;
> >  	case PERF_RECORD_LOST_SAMPLES:
> >  		ret = machine__process_lost_samples_event(machine, event, sample); break;
> >  		break;
> 
> But now you have break; break;

hum, I did not see the PERF_RECORD_LOST_SAMPLES has 2 breaks already,
anyway the one for PERF_RECORD_ITRACE_START is missing

> Isn't putting 'break' on the end of the line making things harder to read?

I guess, but looks like we dont have firm style in this..
attaching v2 with the extra break removal

thanks,
jirka


---
Missing switch break since introduction of new event:
  c4937a91ea56 perf tools: handle PERF_RECORD_LOST_SAMPLES

Also removing unneeded break for PERF_RECORD_LOST_SAMPLES.

Cc: Kan Liang <kan.liang@intel.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Link: http://lkml.kernel.org/n/tip-e3gbrp2561x2s9tkqvf2wh9n@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/machine.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 4744673aff1b..7ff682770fdb 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -1448,10 +1448,9 @@ int machine__process_event(struct machine *machine, union perf_event *event,
 	case PERF_RECORD_AUX:
 		ret = machine__process_aux_event(machine, event); break;
 	case PERF_RECORD_ITRACE_START:
-		ret = machine__process_itrace_start_event(machine, event);
+		ret = machine__process_itrace_start_event(machine, event); break;
 	case PERF_RECORD_LOST_SAMPLES:
 		ret = machine__process_lost_samples_event(machine, event, sample); break;
-		break;
 	default:
 		ret = -1;
 		break;
-- 
1.9.3

Re: [PATCH perf/core] perf tools: Add missing break for PERF_RECORD_ITRACE_START

[Peter Zijlstra]

On Mon, Jun 29, 2015 at 02:17:18PM +0300, Adrian Hunter wrote:
> On 29/06/15 14:12, Jiri Olsa wrote:
> > Missing switch break since introduction of new event:
> >   c4937a91ea56 perf tools: handle PERF_RECORD_LOST_SAMPLES
> > 
> > Cc: Kan Liang <kan.liang@intel.com>
> > Cc: Adrian Hunter <adrian.hunter@intel.com>
> > Link: http://lkml.kernel.org/n/tip-e3gbrp2561x2s9tkqvf2wh9n@git.kernel.org
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  tools/perf/util/machine.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> > index 4744673aff1b..a08e38339cac 100644
> > --- a/tools/perf/util/machine.c
> > +++ b/tools/perf/util/machine.c
> > @@ -1448,7 +1448,7 @@ int machine__process_event(struct machine *machine, union perf_event *event,
> >  	case PERF_RECORD_AUX:
> >  		ret = machine__process_aux_event(machine, event); break;
> >  	case PERF_RECORD_ITRACE_START:
> > -		ret = machine__process_itrace_start_event(machine, event);
> > +		ret = machine__process_itrace_start_event(machine, event); break;
> >  	case PERF_RECORD_LOST_SAMPLES:
> >  		ret = machine__process_lost_samples_event(machine, event, sample); break;
> >  		break;
> 
> But now you have break; break;
> 
> Isn't putting 'break' on the end of the line making things harder to read?

I would tend to agree, its a very odd style.

Re: [PATCH perf/core] perf tools: Add missing break for PERF_RECORD_ITRACE_START

[Arnaldo Carvalho de Melo]

Em Mon, Jun 29, 2015 at 01:47:14PM +0200, Peter Zijlstra escreveu:
> On Mon, Jun 29, 2015 at 02:17:18PM +0300, Adrian Hunter wrote:
> > On 29/06/15 14:12, Jiri Olsa wrote:
> > > +++ b/tools/perf/util/machine.c
> > > @@ -1448,7 +1448,7 @@ int machine__process_event(struct machine *machine, union perf_event *event,
> > >  	case PERF_RECORD_AUX:
> > >  		ret = machine__process_aux_event(machine, event); break;
> > >  	case PERF_RECORD_ITRACE_START:
> > > -		ret = machine__process_itrace_start_event(machine, event);
> > > +		ret = machine__process_itrace_start_event(machine, event); break;
> > >  	case PERF_RECORD_LOST_SAMPLES:
> > >  		ret = machine__process_lost_samples_event(machine, event, sample); break;
> > >  		break;

> > But now you have break; break;

> > Isn't putting 'break' on the end of the line making things harder to read?
 
> I would tend to agree, its a very odd style.

Humm, there are two cases in tools/perf/, where it was made (hey, I am
to blame) for really simple stuff like:

    case A: B(); break;

To make it more compact.

But I just checked and this is not that uncommon:

  [acme@zoo linux]$ find . -name "*.[ch]" | xargs grep ';.*break'  | wc -l
  5035
  [acme@zoo linux]$ find arch -name "*.[ch]" | xargs grep ';.*break' | wc -l
  1440
  [acme@zoo linux]$ find block -name "*.[ch]" | xargs grep ';.*break' | wc -l
  7
  [acme@zoo linux]$ find crypto -name "*.[ch]" | xargs grep ';.*break' | wc -l
  0
  [acme@zoo linux]$ find fs -name "*.[ch]" | xargs grep ';.*break' | wc -l
  66
  [acme@zoo linux]$ find kernel -name "*.[ch]" | xargs grep ';.*break' | wc -l
  11
  [acme@zoo linux]$ find mm -name "*.[ch]" | xargs grep ';.*break' | wc -l
  3
  [acme@zoo linux]$

But I am not that attached to it, as said, just two cases :)

- Arnaldo

Re: [PATCHv2 perf/core] perf tools: Add missing break for PERF_RECORD_ITRACE_START

[Arnaldo Carvalho de Melo]

Em Mon, Jun 29, 2015 at 01:27:45PM +0200, Jiri Olsa escreveu:
> On Mon, Jun 29, 2015 at 02:17:18PM +0300, Adrian Hunter wrote:
> > But now you have break; break;
> 
> hum, I did not see the PERF_RECORD_LOST_SAMPLES has 2 breaks already,
> anyway the one for PERF_RECORD_ITRACE_START is missing
> 
> > Isn't putting 'break' on the end of the line making things harder to read?
> 
> I guess, but looks like we dont have firm style in this..
> attaching v2 with the extra break removal

Thanks, applied.

- Arnaldo

[tip:perf/urgent] perf tools: Add missing break for PERF_RECORD_ITRACE_START

[tip-bot for Jiri Olsa]

Commit-ID:  ceb92913078e41e2305250754e0ea144fc3e9b28
Gitweb:     http://git.kernel.org/tip/ceb92913078e41e2305250754e0ea144fc3e9b28
Author:     Jiri Olsa <jolsa@redhat.com>
AuthorDate: Mon, 29 Jun 2015 13:27:45 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 1 Jul 2015 17:53:48 -0300

perf tools: Add missing break for PERF_RECORD_ITRACE_START

Missing switch break since introduction of new event:

  c4937a91ea56 perf tools: handle PERF_RECORD_LOST_SAMPLES

Also removing unneeded break for PERF_RECORD_LOST_SAMPLES.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/20150629112745.GA21507@krava.brq.redhat.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/machine.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 4744673..7ff6827 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -1448,10 +1448,9 @@ int machine__process_event(struct machine *machine, union perf_event *event,
 	case PERF_RECORD_AUX:
 		ret = machine__process_aux_event(machine, event); break;
 	case PERF_RECORD_ITRACE_START:
-		ret = machine__process_itrace_start_event(machine, event);
+		ret = machine__process_itrace_start_event(machine, event); break;
 	case PERF_RECORD_LOST_SAMPLES:
 		ret = machine__process_lost_samples_event(machine, event, sample); break;
-		break;
 	default:
 		ret = -1;
 		break;