[PATCH 0/2] xenalyze: fixes for gcc-6's -Wmisleading-indentation

[PATCH 0/2] xenalyze: fixes for gcc-6's -Wmisleading-indentation

[Ian Campbell]

Debian bug 812166[0] concerned failures due to -Werror=misleading-
indentation when building the Xen package.

While trying (and failing) to reproduce those failures I came across these
two warnings in xenalyze, one relating to misleading indenation and the
other for unused code.

Cheers,
Ian.

[0] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=812166

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH 1/2] xenalyze: fix misleading indentation.

[Ian Campbell]

gcc-6 adds -Wmisleading-indentation which found these issues.

xenalyze.c: In function 'weighted_percentile':
xenalyze.c:2136:18: error: statement is indented as if it were guarded by... [-Werror=misleading-indentation]
             L=I; L_weight = I_weight;
                  ^~~~~~~~

xenalyze.c:2135:9: note: ...this 'if' clause, but it is not
         if(J_weight<K_weight)
         ^~

xenalyze.c:2138:18: error: statement is indented as if it were guarded by... [-Werror=misleading-indentation]
             R=J; R_weight = J_weight;
                  ^~~~~~~~

xenalyze.c:2137:9: note: ...this 'if' clause, but it is not
         if(K_weight<I_weight)
         ^~

xenalyze.c: In function 'self_weighted_percentile':
xenalyze.c:2215:18: error: statement is indented as if it were guarded by... [-Werror=misleading-indentation]
             L=I; L_weight = I_weight;
                  ^~~~~~~~

xenalyze.c:2214:9: note: ...this 'if' clause, but it is not
         if(J_weight<K_weight)
         ^~

xenalyze.c:2217:18: error: statement is indented as if it were guarded by... [-Werror=misleading-indentation]
             R=J; R_weight = J_weight;
                  ^~~~~~~~

xenalyze.c:2216:9: note: ...this 'if' clause, but it is not
         if(K_weight<I_weight)
         ^~

I've modified according to what I think the intention is, i.e. added braces
rather than moving the line in question out a level.

I have only build tested the result.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 tools/xentrace/xenalyze.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/tools/xentrace/xenalyze.c b/tools/xentrace/xenalyze.c
index 5a2735c..4bcaf83 100644
--- a/tools/xentrace/xenalyze.c
+++ b/tools/xentrace/xenalyze.c
@@ -2132,10 +2132,14 @@ float weighted_percentile(float * A, /* values */
         } while (I <= J); /* Keep going until our pointers meet or pass */
 
         /* Re-adjust L and R, based on which element we're looking for */
-        if(J_weight<K_weight)
-            L=I; L_weight = I_weight;
-        if(K_weight<I_weight)
-            R=J; R_weight = J_weight;
+        if(J_weight<K_weight) {
+            L=I;
+            L_weight = I_weight;
+        }
+        if(K_weight<I_weight) {
+            R=J;
+            R_weight = J_weight;
+        }
     }
 
     return A[L];
@@ -2211,10 +2215,14 @@ long long self_weighted_percentile(long long * A,
         } while (I <= J); /* Keep going until our pointers meet or pass */
 
         /* Re-adjust L and R, based on which element we're looking for */
-        if(J_weight<K_weight)
-            L=I; L_weight = I_weight;
-        if(K_weight<I_weight)
-            R=J; R_weight = J_weight;
+        if(J_weight<K_weight) {
+            L=I;
+            L_weight = I_weight;
+        }
+        if(K_weight<I_weight) {
+            R=J;
+            R_weight = J_weight;
+        }
     }
 
     return A[L];
-- 
2.6.1

Re: [PATCH 1/2] xenalyze: fix misleading indentation.

[George Dunlap]

On 22/01/16 14:27, Ian Campbell wrote:
> gcc-6 adds -Wmisleading-indentation which found these issues.
> 
> xenalyze.c: In function 'weighted_percentile':
> xenalyze.c:2136:18: error: statement is indented as if it were guarded by... [-Werror=misleading-indentation]
>              L=I; L_weight = I_weight;
>                   ^~~~~~~~
> 
> xenalyze.c:2135:9: note: ...this 'if' clause, but it is not
>          if(J_weight<K_weight)
>          ^~
> 
> xenalyze.c:2138:18: error: statement is indented as if it were guarded by... [-Werror=misleading-indentation]
>              R=J; R_weight = J_weight;
>                   ^~~~~~~~
> 
> xenalyze.c:2137:9: note: ...this 'if' clause, but it is not
>          if(K_weight<I_weight)
>          ^~
> 
> xenalyze.c: In function 'self_weighted_percentile':
> xenalyze.c:2215:18: error: statement is indented as if it were guarded by... [-Werror=misleading-indentation]
>              L=I; L_weight = I_weight;
>                   ^~~~~~~~
> 
> xenalyze.c:2214:9: note: ...this 'if' clause, but it is not
>          if(J_weight<K_weight)
>          ^~
> 
> xenalyze.c:2217:18: error: statement is indented as if it were guarded by... [-Werror=misleading-indentation]
>              R=J; R_weight = J_weight;
>                   ^~~~~~~~
> 
> xenalyze.c:2216:9: note: ...this 'if' clause, but it is not
>          if(K_weight<I_weight)
>          ^~
> 
> I've modified according to what I think the intention is, i.e. added braces
> rather than moving the line in question out a level.
> 
> I have only build tested the result.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

!!!

Hrm, well obviously the "weighted percentiles" have been wonky for a
long time. :-/

Reviewed-by: George Dunlap <george.dunlap@citrix.com>

[PATCH 2/2] xenalyze: remove cr3_compare_total

[Ian Campbell]

gcc-6 complains:
xenalyze.c:4132:9: error: 'cr3_compare_total' defined but not used [-Werror=unused-function]
     int cr3_compare_total(const void *_a, const void *_b) {
         ^~~~~~~~~~~~~~~~~

I believe it is correct.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 tools/xentrace/xenalyze.c | 17 -----------------
 1 file changed, 17 deletions(-)

diff --git a/tools/xentrace/xenalyze.c b/tools/xentrace/xenalyze.c
index 4bcaf83..6520790 100644
--- a/tools/xentrace/xenalyze.c
+++ b/tools/xentrace/xenalyze.c
@@ -4129,23 +4129,6 @@ void cr3_dump_list(struct cr3_value_struct *head){
     struct cr3_value_struct **qsort_array;
     int i, N=0;
 
-    int cr3_compare_total(const void *_a, const void *_b) {
-        struct cr3_value_struct *a=*(typeof(&a))_a;
-        struct cr3_value_struct *b=*(typeof(&a))_b;
-
-        if(a->total_time.cycles < b->total_time.cycles)
-            return 1;
-        else if(b->total_time.cycles == a->total_time.cycles) {
-            if(a->total_time.count < b->total_time.count)
-                return 1;
-            else if(a->total_time.count == b->total_time.count)
-                return 0;
-            else
-                return -1;
-        } else
-            return -1;
-    }
-
     int cr3_compare_start(const void *_a, const void *_b) {
         struct cr3_value_struct *a=*(typeof(&a))_a;
         struct cr3_value_struct *b=*(typeof(&a))_b;
-- 
2.6.1

Re: [PATCH 2/2] xenalyze: remove cr3_compare_total

[George Dunlap]

On 22/01/16 14:27, Ian Campbell wrote:
> gcc-6 complains:
> xenalyze.c:4132:9: error: 'cr3_compare_total' defined but not used [-Werror=unused-function]
>      int cr3_compare_total(const void *_a, const void *_b) {
>          ^~~~~~~~~~~~~~~~~
> 
> I believe it is correct.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

Thanks,

Reviewed-by: George Dunlap <george.dunlap@citrix.com>

Re: [PATCH 0/2] xenalyze: fixes for gcc-6's -Wmisleading-indentation

[Ian Campbell]

On Fri, 2016-01-22 at 14:27 +0000, Ian Campbell wrote:
> Debian bug 812166[0] concerned failures due to -Werror=misleading-
> indentation when building the Xen package.
> 
> While trying (and failing) to reproduce those failures I came across
> these
> two warnings in xenalyze, one relating to misleading indenation and the
> other for unused code.

Applied both with George's R-by.