From: Gerhard Sittig <redacted>
Date: Sun, 17 May 2020 14:50:42 +0000 (+0200)
Subject: asix-sigma: force strict boolen arith in LUT item manipulation
X-Git-Url: https://sigrok.org/gitweb/?p=libsigrok.git;a=commitdiff_plain;h=ea57157d0d2e17e418463cf8e4f25453e5b903d7

asix-sigma: force strict boolen arith in LUT item manipulation

Mechanically adjust the add_trigger_function() routine to address nits,
attempt to improve maintainability.

Raise awareness of the fact that strict binary arithmetics is done (bit
operators are used), the strict 0..1 set of values needs to be enforced,
and mere "logical truthness" is not good enough in this spot. Explicitly
check for bit positions instead of "shifting out" the bit of interest
and have the 0/1 value result nearly by coincidence.

Extend comments. Group related instructions and separate them from other
groups. Reduce the scope of the rather generic i, j, tmp named variables
which are just too easy to get wrong.
---

diff --git a/src/hardware/asix-sigma/protocol.c b/src/hardware/asix-sigma/protocol.c
index e913e258..e87795e8 100644
--- a/src/hardware/asix-sigma/protocol.c
+++ b/src/hardware/asix-sigma/protocol.c
@@ -1839,12 +1839,17 @@ static void build_lut_entry(uint16_t *lut_entry,
 static void add_trigger_function(enum triggerop oper, enum triggerfunc func,
 	size_t index, gboolean neg, uint16_t *mask)
 {
-	size_t i, j;
-	int x[2][2], tmp, a, b, aset, bset, rset;
+	int x[2][2], a, b, aset, bset, rset;
+	size_t bitidx;
 
-	memset(x, 0, sizeof(x));
+	/*
+	 * Beware! The x, a, b, aset, bset, rset variables strictly
+	 * require the limited 0..1 range. They are not interpreted
+	 * as logically true, instead bit arith is done on them.
+	 */
 
-	/* Trigger detect condition. */
+	/* Construct a pattern which detects the condition. */
+	memset(x, 0, sizeof(x));
 	switch (oper) {
 	case OP_LEVEL:
 		x[0][1] = 1;
@@ -1880,8 +1885,11 @@ static void add_trigger_function(enum triggerop oper, enum triggerfunc func,
 		break;
 	}
 
-	/* Transpose if neg is set. */
+	/* Transpose the pattern if the condition is negated. */
 	if (neg) {
+		size_t i, j;
+		int tmp;
+
 		for (i = 0; i < 2; i++) {
 			for (j = 0; j < 2; j++) {
 				tmp = x[i][j];
@@ -1891,29 +1899,30 @@ static void add_trigger_function(enum triggerop oper, enum triggerfunc func,
 		}
 	}
 
-	/* Update mask with function. */
-	for (i = 0; i < 16; i++) {
-		a = (i >> (2 * index + 0)) & 1;
-		b = (i >> (2 * index + 1)) & 1;
+	/* Update the LUT mask with the function's condition. */
+	for (bitidx = 0; bitidx < 16; bitidx++) {
+		a = (bitidx & BIT(2 * index + 0)) ? 1 : 0;
+		b = (bitidx & BIT(2 * index + 1)) ? 1 : 0;
 
-		aset = (*mask >> i) & 1;
+		aset = (*mask & BIT(bitidx)) ? 1 : 0;
 		bset = x[b][a];
 
-		rset = 0;
 		if (func == FUNC_AND || func == FUNC_NAND)
 			rset = aset & bset;
 		else if (func == FUNC_OR || func == FUNC_NOR)
 			rset = aset | bset;
 		else if (func == FUNC_XOR || func == FUNC_NXOR)
 			rset = aset ^ bset;
+		else
+			rset = 0;
 
 		if (func == FUNC_NAND || func == FUNC_NOR || func == FUNC_NXOR)
-			rset = !rset;
-
-		*mask &= ~BIT(i);
+			rset = 1 - rset;
 
 		if (rset)
-			*mask |= BIT(i);
+			*mask |= BIT(bitidx);
+		else
+			*mask &= ~BIT(bitidx);
 	}
 }