Fix misleading indentation issues in perf

PATCH In review Markus Trippelsdorf 5 participants 11 messages Dec 12, 2015 – Feb 3, 2016 perf events

lore

Share & tools

lore

Markus Trippelsdorf

author

2015-12-12 18:07 UTC #0 lore raw

PATCH 1/1 Fix misleading indentation issues in perf

perf doesn't build with gcc-6 because of several misleading-indentation warnings, e.g.:

arch/x86/tests/intel-cqm.c: In function ‘spawn’:
arch/x86/tests/intel-cqm.c:21:3: error: statement is indented as if it were guarded by... [-Werror=misleading-indentation]
sleep(5);
^~~~~

arch/x86/tests/intel-cqm.c:20:2: note: ...this ‘while’ clause, but it is not
while(1);
^~~~~

Signed-off-by Markus Trippelsdorf <[email protected]>

tools/perf/arch/x86/tests/intel-cqm.c +1 -1

index d28c1b6a3b54..fa5d17af88b7 100644
@@ -17,7 +17,7 @@ static pid_t spawn(void)
 	if (pid)
 		return pid;
 
-	while(1);
+	while(1)
 		sleep(5);
 	return 0;
 }

tools/perf/ui/browsers/annotate.c +2 -2

index d4d7cc27252f..718bd46d47fa 100644
@@ -755,11 +755,11 @@ static int annotate_browser__run(struct annotate_browser *browser,
 				nd = browser->curr_hot;
 			break;
 		case K_UNTAB:
-			if (nd != NULL)
+			if (nd != NULL) {
 				nd = rb_next(nd);
 				if (nd == NULL)
 					nd = rb_first(&browser->entries);
-			else
+			} else
 				nd = browser->curr_hot;
 			break;
 		case K_F1:

tools/perf/util/pmu.c +1 -1

index e4b173dec4b9..c900b664ab8f 100644
@@ -153,7 +153,7 @@ static int perf_pmu__parse_unit(struct perf_pmu_alias *alias, char *dir, char *n
 	if (fd == -1)
 		return -1;
 
-		sret = read(fd, alias->unit, UNIT_MAX_LEN);
+	sret = read(fd, alias->unit, UNIT_MAX_LEN);
 	if (sret < 0)
 		goto error;

Signature · 1 line

Markus

Ingo Molnar

2015-12-14 08:20 UTC #1 lore raw

Quoted 11 lines · Markus Trippelsdorf

perf doesn't build with gcc-6 because of several misleading-indentation warnings, e.g.:
arch/x86/tests/intel-cqm.c: In function ‘spawn’:
arch/x86/tests/intel-cqm.c:21:3: error: statement is indented as if it were guarded by... [-Werror=misleading-indentation]
   sleep(5);
   ^~~~~
arch/x86/tests/intel-cqm.c:20:2: note: ...this ‘while’ clause, but it is not
while(1);
^~~~~

Nice warnings!

tools/perf/arch/x86/tests/intel-cqm.c static pid_t spawn(void) · line 21

Quoted 11 lines

tools/perf/arch/x86/tests/intel-cqm.c +1 -1

index d28c1b6a3b54..fa5d17af88b7 100644
@@ -17,7 +17,7 @@ static pid_t spawn(void)
 	if (pid)
 		return pid;
 
-	while(1);
+	while(1)
 		sleep(5);

looks like it caught a real bug here.

tools/perf/arch/x86/tests/intel-cqm.c

Quoted 10 lines

tools/perf/arch/x86/tests/intel-cqm.c +1 -1

 		case K_UNTAB:
-			if (nd != NULL)
+			if (nd != NULL) {

tools/perf/arch/x86/tests/intel-cqm.c +1 -1

 				nd = rb_next(nd);
 				if (nd == NULL)
 					nd = rb_first(&browser->entries);
-			else
+			} else
 				nd = browser->curr_hot;
 			break;

That looks like a real bug as well AFAICS.

tools/perf/util/pmu.c static int perf_pmu__parse_unit(struct perf_pmu_alias *alias, char *dir, char *n · line 158

Quoted 13 lines

tools/perf/arch/x86/tests/intel-cqm.c +0 -0

 		case K_F1:

tools/perf/util/pmu.c +1 -1

index e4b173dec4b9..c900b664ab8f 100644
@@ -153,7 +153,7 @@ static int perf_pmu__parse_unit(struct perf_pmu_alias *alias, char *dir, char *n
 	if (fd == -1)
 		return -1;
 
-		sret = read(fd, alias->unit, UNIT_MAX_LEN);
+	sret = read(fd, alias->unit, UNIT_MAX_LEN);
 	if (sret < 0)
 		goto error;

This is just whitespace noise, but annoying.

Cool compiler feature.

Thanks,

Ingo

Matt Fleming

Reviewed-by on v1 1/1

2015-12-14 10:46 UTC #2 lore raw

In reply to Markus Trippelsdorf

Quoted 19 lines · Markus Trippelsdorf

perf doesn't build with gcc-6 because of several misleading-indentation warnings, e.g.:
arch/x86/tests/intel-cqm.c: In function ‘spawn’:
arch/x86/tests/intel-cqm.c:21:3: error: statement is indented as if it were guarded by... [-Werror=misleading-indentation]
   sleep(5);
   ^~~~~
arch/x86/tests/intel-cqm.c:20:2: note: ...this ‘while’ clause, but it is not
while(1);
^~~~~
Signed-off-by Markus Trippelsdorf <[email protected]>
tools/perf/arch/x86/tests/intel-cqm.c +0 -0
index d28c1b6a3b54..fa5d17af88b7 100644

tools/perf/arch/x86/tests/intel-cqm.c static pid_t spawn(void) · line 23

Quoted 8 lines

tools/perf/arch/x86/tests/intel-cqm.c +1 -1

@@ -17,7 +17,7 @@ static pid_t spawn(void)
 	if (pid)
 		return pid;
 
-	while(1);
+	while(1)
 		sleep(5);
 	return 0;
 }

Whoops. Good catch.

Reviewed-by Matt Fleming <[email protected]>

Arnaldo Carvalho de Melo

maintainer

2015-12-14 15:14 UTC #3 lore raw

Em Mon, Dec 14, 2015 at 10:46:07AM +0000, Matt Fleming escreveu:

Quoted 31 lines · Markus Trippelsdorf

Quoted 19 lines
perf doesn't build with gcc-6 because of several misleading-indentation warnings, e.g.:
arch/x86/tests/intel-cqm.c: In function ‘spawn’:
arch/x86/tests/intel-cqm.c:21:3: error: statement is indented as if it were guarded by... [-Werror=misleading-indentation]
   sleep(5);
   ^~~~~
arch/x86/tests/intel-cqm.c:20:2: note: ...this ‘while’ clause, but it is not
while(1);
^~~~~
Signed-off-by Markus Trippelsdorf <[email protected]>
tools/perf/arch/x86/tests/intel-cqm.c +0 -0
index d28c1b6a3b54..fa5d17af88b7 100644
tools/perf/arch/x86/tests/intel-cqm.c static pid_t spawn(void) · line 23
Quoted 8 lines
tools/perf/arch/x86/tests/intel-cqm.c +1 -1
@@ -17,7 +17,7 @@ static pid_t spawn(void)
 	if (pid)
 		return pid;
 
-	while(1);
+	while(1)
 		sleep(5);
 	return 0;
 }
Whoops. Good catch.

Quoted 1 line

Reviewed-by Matt Fleming <[email protected]>

So, Markus, can you split this in three patches, stating that in some cases its just cosmetic stuff while in others really a bug got fixed, adding the Reviewed-by: tag for the cqm one? And acked-by for all, from Ingo?

- Arnaldo

Markus Trippelsdorf

author

2015-12-14 15:43 UTC #4 lore raw

PATCH 1/3 Remove wrong semicolon in while loop

The while loop was spinning. Fix by removing a semicolon.

The issue was pointed out by gcc-6's -Wmisleading-indentation.

Reviewed-by Matt Fleming <[email protected]>

Acked-by Ingo Molnar <[email protected]>

Signed-off-by Markus Trippelsdorf <[email protected]>

tools/perf/arch/x86/tests/intel-cqm.c +1 -1

index d28c1b6a3b54..fa5d17af88b7 100644
@@ -17,7 +17,7 @@ static pid_t spawn(void)
 	if (pid)
 		return pid;
 
-	while(1);
+	while(1)
 		sleep(5);
 	return 0;
 }

Signature · 1 line

Markus

Arnaldo Carvalho de Melo

maintainer

2015-12-14 17:55 UTC #5 lore raw

Re: [PATCH 1/3] Remove wrong semicolon in while loop

Em Mon, Dec 14, 2015 at 04:43:35PM +0100, Markus Trippelsdorf escreveu:

Quoted 1 line

The while loop was spinning. Fix by removing a semicolon.

Thanks for splitting it up!

- Arnaldo

tip-bot for Markus Trippelsdorf

merge bot

2016-01-30 08:25 UTC #6 lore raw

[tip:perf/urgent] perf tests: Remove wrong semicolon in while loop in CQM test

In reply to Markus Trippelsdorf

perf tests: Remove wrong semicolon in while loop in CQM test

Commit-ID:  cf89813a5b514bff9b3b5e7eaf2090f22fba62e0
Gitweb:     http://git.kernel.org/tip/cf89813a5b514bff9b3b5e7eaf2090f22fba62e0
Author:     Markus Trippelsdorf <[email protected]>
AuthorDate: Mon, 14 Dec 2015 16:43:35 +0100
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 26 Jan 2016 11:14:06 -0300

perf tests: Remove wrong semicolon in while loop in CQM test

The while loop was spinning. Fix by removing a semicolon.

The issue was pointed out by gcc-6's -Wmisleading-indentation.

Signed-off-by Markus Trippelsdorf <[email protected]>

Reviewed-by Matt Fleming <[email protected]>

Acked-by Ingo Molnar <[email protected]>

Cc Ben Hutchings <[email protected]>

Cc Peter Zijlstra <[email protected]>

Fixes 035827e9f2bd ("perf tests: Add Intel CQM test")

Link http://lkml.kernel.org/r/20151214154335.GA1409@x4 open here

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 tools/perf/arch/x86/tests/intel-cqm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

tools/perf/arch/x86/tests/intel-cqm.c +1 -1

index 3e89ba8..7f064eb 100644
@@ -17,7 +17,7 @@ static pid_t spawn(void)
 	if (pid)
 		return pid;
 
-	while(1);
+	while(1)
 		sleep(5);
 	return 0;
 }

Markus Trippelsdorf

author

2015-12-14 15:44 UTC #7 lore raw

PATCH 2/3 Add missing braces to if statement

In reply to Arnaldo Carvalho de Melo

Add missing braces to if statement.

The issue was pointed out by gcc-6's -Wmisleading-indentation.

Acked-by Ingo Molnar <[email protected]>

Signed-off-by Markus Trippelsdorf <[email protected]>

tools/perf/ui/browsers/annotate.c +2 -2

index d4d7cc27252f..718bd46d47fa 100644
@@ -755,11 +755,11 @@ static int annotate_browser__run(struct annotate_browser *browser,
 				nd = browser->curr_hot;
 			break;
 		case K_UNTAB:
-			if (nd != NULL)
+			if (nd != NULL) {
 				nd = rb_next(nd);
 				if (nd == NULL)
 					nd = rb_first(&browser->entries);
-			else
+			} else
 				nd = browser->curr_hot;
 			break;
 		case K_F1:

Signature · 1 line

Markus

tip-bot for Markus Trippelsdorf

merge bot

2016-01-30 08:25 UTC #8 lore raw

[tip:perf/urgent] perf annotate browser: Fix behaviour of Shift-Tab with nothing focussed

perf annotate browser: Fix behaviour of Shift-Tab with nothing focussed

Commit-ID:  d4913cbd05bab685e49c8174896e563b2487d054
Gitweb:     http://git.kernel.org/tip/d4913cbd05bab685e49c8174896e563b2487d054
Author:     Markus Trippelsdorf <[email protected]>
AuthorDate: Mon, 14 Dec 2015 16:44:03 +0100
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 26 Jan 2016 11:14:25 -0300

perf annotate browser: Fix behaviour of Shift-Tab with nothing focussed

The issue was pointed out by gcc-6's -Wmisleading-indentation.

Signed-off-by Markus Trippelsdorf <[email protected]>

Acked-by Ingo Molnar <[email protected]>

Cc Ben Hutchings <[email protected]>

Cc Matt Fleming <[email protected]>

Cc Peter Zijlstra <[email protected]>

Fixes c97cf42219b7 ("perf top: Live TUI Annotation")

Link http://lkml.kernel.org/r/20151214154403.GB1409@x4 open here

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 tools/perf/ui/browsers/annotate.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

tools/perf/ui/browsers/annotate.c +2 -2

index d4d7cc2..718bd46 100644
@@ -755,11 +755,11 @@ static int annotate_browser__run(struct annotate_browser *browser,
 				nd = browser->curr_hot;
 			break;
 		case K_UNTAB:
-			if (nd != NULL)
+			if (nd != NULL) {
 				nd = rb_next(nd);
 				if (nd == NULL)
 					nd = rb_first(&browser->entries);
-			else
+			} else
 				nd = browser->curr_hot;
 			break;
 		case K_F1:

Markus Trippelsdorf

author

2015-12-14 15:44 UTC #9 lore raw

PATCH 3/3 Fix misleadingly indented assignment (whitespace)

In reply to Arnaldo Carvalho de Melo

Fix misleadingly indented assignment.
This is just a simple whitespace fix.

The issue was pointed out by gcc-6's -Wmisleading-indentation.

Acked-by Ingo Molnar <[email protected]>

Signed-off-by Markus Trippelsdorf <[email protected]>

tools/perf/util/pmu.c +1 -1

index e4b173dec4b9..c900b664ab8f 100644
@@ -153,7 +153,7 @@ static int perf_pmu__parse_unit(struct perf_pmu_alias *alias, char *dir, char *n
 	if (fd == -1)
 		return -1;
 
-		sret = read(fd, alias->unit, UNIT_MAX_LEN);
+	sret = read(fd, alias->unit, UNIT_MAX_LEN);
 	if (sret < 0)
 		goto error;

Signature · 1 line

Markus

tip-bot for Markus Trippelsdorf

merge bot

2016-02-03 10:07 UTC #10 lore raw

[tip:perf/core] perf pmu: Fix misleadingly indented assignment ( whitespace)

perf pmu: Fix misleadingly indented assignment (whitespace)

Commit-ID:  d85ce830eef6c10d1e9617172dea4681f02b8424
Gitweb:     http://git.kernel.org/tip/d85ce830eef6c10d1e9617172dea4681f02b8424
Author:     Markus Trippelsdorf <[email protected]>
AuthorDate: Mon, 14 Dec 2015 16:44:40 +0100
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 26 Jan 2016 11:52:42 -0300

perf pmu: Fix misleadingly indented assignment (whitespace)

One line in perf_pmu__parse_unit() is indented wrongly, leading to a warning (=> error) from gcc 6:

util/pmu.c:156:3: error: statement is indented as if it were guarded by... [-Werror=misleading-indentation]

    sret = read(fd, alias->unit, UNIT_MAX_LEN);
    ^~~~

util/pmu.c:153:2: note: ...this 'if' clause, but it is not

    if (fd == -1)
    ^~

Signed-off-by Markus Trippelsdorf <[email protected]>

Acked-by Ingo Molnar <[email protected]>

Cc Ben Hutchings <[email protected]>

Cc Matt Fleming <[email protected]>

Cc Peter Zijlstra <[email protected]>

Fixes 410136f5dd96 ("tools/perf/stat: Add event unit and scale support")

Link http://lkml.kernel.org/r/20151214154440.GC1409@x4 open here

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 tools/perf/util/pmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

tools/perf/util/pmu.c +1 -1

index b597bcc..41a9c87 100644
@@ -153,7 +153,7 @@ static int perf_pmu__parse_unit(struct perf_pmu_alias *alias, char *dir, char *n
 	if (fd == -1)
 		return -1;
 
-		sret = read(fd, alias->unit, UNIT_MAX_LEN);
+	sret = read(fd, alias->unit, UNIT_MAX_LEN);
 	if (sret < 0)
 		goto error;