Change in ...coreboot[master]: util/inteltool: Swap conditions to prevent uninit reads

[Jacob Garber (Code Review)]

Jacob Garber has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32269

Change subject: util/inteltool: Swap conditions to prevent uninit reads
......................................................................

util/inteltool: Swap conditions to prevent uninit reads

Both values in each array are only initialized if
`two_channels` is true, so we need to check that first.

Found-by: Coverity Scan #1370{584,585,588,589,590-596,600}
Signed-off-by: Jacob Garber 
Change-Id: I592bc6ae00f834f74a61668d7a3919014ec635f3
---
M util/inteltool/ivy_memory.c
1 file changed, 12 insertions(+), 12 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/32269/1

diff --git a/util/inteltool/ivy_memory.c b/util/inteltool/ivy_memory.c
index f0e473f..8ce8611 100644
--- a/util/inteltool/ivy_memory.c
+++ b/util/inteltool/ivy_memory.c
@@ -172,47 +172,47 @@

printf(".reg_4004_b30 = { %d, %d },\n", reg_4004_b30[0],
           reg_4004_b30[1]);
-	if (tFAW[0] != tFAW[1] && two_channels)
+	if (two_channels && tFAW[0] != tFAW[1])
    	printf("/* tFAW mismatch: %d, %d */\n", tFAW[0], tFAW[1]);
    print_time("tFAW", tFAW[0], tCK);
-	if (tWTR[0] != tWTR[1] && two_channels)
+	if (two_channels && tWTR[0] != tWTR[1])
    	printf("/* tWTR mismatch: %d, %d */\n", tWTR[0], tWTR[1]);
    print_time("tWTR", tWTR[0], tCK);
-	if (tCKE[0] != tCKE[1] && two_channels)
+	if (two_channels && tCKE[0] != tCKE[1])
    	printf("/* tCKE mismatch: %d, %d */\n", tCKE[0], tCKE[1]);
    print_time("tCKE", tCKE[0], tCK);
-	if (tRTP[0] != tRTP[1] && two_channels)
+	if (two_channels && tRTP[0] != tRTP[1])
    	printf("/* tRTP mismatch: %d, %d */\n", tRTP[0], tRTP[1]);
    print_time("tRTP", tRTP[0], tCK);
-	if (tRRD[0] != tRRD[1] && two_channels)
+	if (two_channels && tRRD[0] != tRRD[1])
    	printf("/* tRRD mismatch: %d, %d */\n", tRRD[0], tRRD[1]);
    print_time("tRRD", tRRD[0], tCK);

-	if (tRAS[0] != tRAS[1] && two_channels)
+	if (two_channels && tRAS[0] != tRAS[1])
    	printf("/* tRAS mismatch: %d, %d */\n", tRAS[0], tRAS[1]);
    print_time("tRAS", tRAS[0], tCK);

-	if (tCWL[0] != tCWL[1] && two_channels)
+	if (two_channels && tCWL[0] != tCWL[1])
    	printf("/* tCWL mismatch: %d, %d */\n", tCWL[0], tCWL[1]);
    print_time("tCWL", tCWL[0], tCK);

-	if (tRP[0] != tRP[1] && two_channels)
+	if (two_channels && tRP[0] != tRP[1])
    	printf("/* tRP mismatch: %d, %d */\n", tRP[0], tRP[1]);
    print_time("tRP", tRP[0], tCK);

-	if (tRCD[0] != tRCD[1] && two_channels)
+	if (two_channels && tRCD[0] != tRCD[1])
    	printf("/* tRCD mismatch: %d, %d */\n", tRCD[0], tRCD[1]);
    print_time("tRCD", tRCD[0], tCK);

-	if (tXPDLL[0] != tXPDLL[1] && two_channels)
+	if (two_channels && tXPDLL[0] != tXPDLL[1])
    	printf("/* tXPDLL mismatch: %d, %d */\n", tXPDLL[0], tXPDLL[1]);
    print_time("tXPDLL", tXPDLL[0], tCK);

-	if (tXP[0] != tXP[1] && two_channels)
+	if (two_channels && tXP[0] != tXP[1])
    	printf("/* tXP mismatch: %d, %d */\n", tXP[0], tXP[1]);
    print_time("tXP", tXP[0], tCK);

-	if (tAONPD[0] != tAONPD[1] && two_channels)
+	if (two_channels && tAONPD[0] != tAONPD[1])
    	printf("/* tAONPD mismatch: %d, %d */\n", tAONPD[0], tAONPD[1]);
    print_time("tAONPD", tAONPD[0], tCK);

[Patrick Rudolph (Code Review)]

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32269 )

Change subject: util/inteltool: Swap conditions to prevent uninit reads
......................................................................

Patch Set 1:

Looking at the code I don't understand this change.

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32269 )

Change subject: util/inteltool: Swap conditions to prevent uninit reads
......................................................................

Patch Set 1: Code-Review+2
> Patch Set 1:
>
> Looking at the code I don't understand this change.

I think the commit message is fine, after change, right-hand side of boolean AND will only get evaluted when both arrays are initialized.

[Patrick Rudolph (Code Review)]

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32269 )

Change subject: util/inteltool: Swap conditions to prevent uninit reads
......................................................................

Patch Set 1: Code-Review+2

Ok, got it.

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32269 )

Change subject: util/inteltool: Swap conditions to prevent uninit reads
......................................................................

util/inteltool: Swap conditions to prevent uninit reads

Both values in each array are only initialized if
`two_channels` is true, so we need to check that first.

Found-by: Coverity Scan #1370{584,585,588,589,590-596,600}
Signed-off-by: Jacob Garber 
Change-Id: I592bc6ae00f834f74a61668d7a3919014ec635f3
Reviewed-on: https://review.coreboot.org/c/coreboot/+/32269
Tested-by: build bot (Jenkins) 
Reviewed-by: Kyösti Mälkki 
Reviewed-by: Patrick Rudolph 
---
M util/inteltool/ivy_memory.c
1 file changed, 12 insertions(+), 12 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Kyösti Mälkki: Looks good to me, approved
  Patrick Rudolph: Looks good to me, approved

diff --git a/util/inteltool/ivy_memory.c b/util/inteltool/ivy_memory.c
index f0e473f..8ce8611 100644
--- a/util/inteltool/ivy_memory.c
+++ b/util/inteltool/ivy_memory.c
@@ -172,47 +172,47 @@

printf(".reg_4004_b30 = { %d, %d },\n", reg_4004_b30[0],
           reg_4004_b30[1]);
-	if (tFAW[0] != tFAW[1] && two_channels)
+	if (two_channels && tFAW[0] != tFAW[1])
    	printf("/* tFAW mismatch: %d, %d */\n", tFAW[0], tFAW[1]);
    print_time("tFAW", tFAW[0], tCK);
-	if (tWTR[0] != tWTR[1] && two_channels)
+	if (two_channels && tWTR[0] != tWTR[1])
    	printf("/* tWTR mismatch: %d, %d */\n", tWTR[0], tWTR[1]);
    print_time("tWTR", tWTR[0], tCK);
-	if (tCKE[0] != tCKE[1] && two_channels)
+	if (two_channels && tCKE[0] != tCKE[1])
    	printf("/* tCKE mismatch: %d, %d */\n", tCKE[0], tCKE[1]);
    print_time("tCKE", tCKE[0], tCK);
-	if (tRTP[0] != tRTP[1] && two_channels)
+	if (two_channels && tRTP[0] != tRTP[1])
    	printf("/* tRTP mismatch: %d, %d */\n", tRTP[0], tRTP[1]);
    print_time("tRTP", tRTP[0], tCK);
-	if (tRRD[0] != tRRD[1] && two_channels)
+	if (two_channels && tRRD[0] != tRRD[1])
    	printf("/* tRRD mismatch: %d, %d */\n", tRRD[0], tRRD[1]);
    print_time("tRRD", tRRD[0], tCK);

-	if (tRAS[0] != tRAS[1] && two_channels)
+	if (two_channels && tRAS[0] != tRAS[1])
    	printf("/* tRAS mismatch: %d, %d */\n", tRAS[0], tRAS[1]);
    print_time("tRAS", tRAS[0], tCK);

-	if (tCWL[0] != tCWL[1] && two_channels)
+	if (two_channels && tCWL[0] != tCWL[1])
    	printf("/* tCWL mismatch: %d, %d */\n", tCWL[0], tCWL[1]);
    print_time("tCWL", tCWL[0], tCK);

-	if (tRP[0] != tRP[1] && two_channels)
+	if (two_channels && tRP[0] != tRP[1])
    	printf("/* tRP mismatch: %d, %d */\n", tRP[0], tRP[1]);
    print_time("tRP", tRP[0], tCK);

-	if (tRCD[0] != tRCD[1] && two_channels)
+	if (two_channels && tRCD[0] != tRCD[1])
    	printf("/* tRCD mismatch: %d, %d */\n", tRCD[0], tRCD[1]);
    print_time("tRCD", tRCD[0], tCK);

-	if (tXPDLL[0] != tXPDLL[1] && two_channels)
+	if (two_channels && tXPDLL[0] != tXPDLL[1])
    	printf("/* tXPDLL mismatch: %d, %d */\n", tXPDLL[0], tXPDLL[1]);
    print_time("tXPDLL", tXPDLL[0], tCK);

-	if (tXP[0] != tXP[1] && two_channels)
+	if (two_channels && tXP[0] != tXP[1])
    	printf("/* tXP mismatch: %d, %d */\n", tXP[0], tXP[1]);
    print_time("tXP", tXP[0], tCK);

-	if (tAONPD[0] != tAONPD[1] && two_channels)
+	if (two_channels && tAONPD[0] != tAONPD[1])
    	printf("/* tAONPD mismatch: %d, %d */\n", tAONPD[0], tAONPD[1]);
    print_time("tAONPD", tAONPD[0], tCK);