Index | Thread | Search

From:
Claudio Jeker <claudio@openbsd.org>
Subject:
ctfconv fix it_cmp function
To:
Martin Pieuchot <mpi@openbsd.org>
Cc:
tech@openbsd.org
Date:
Thu, 22 Feb 2024 12:11:56 +0100

Download raw body.

Thread
C integer promotion bytes again. In many cases you can't use a pattern
like:
	int diff;
	if ((diff = (a->it_type - b->it_type)) != 0)
		return diff;

The type of it_type matters and the result may not be what you think.

Use the pattern with explicit > and < tests instead.
With this the problem of duplicated types in the CTF is gone since the RB
tree is finally in order.

I'm not smart enough to know when (diff = (a - b)) is safe and when it
isn't so I just converted them all.

On top of this add a check for the encoding for CTF_K_INTEGER and
CTF_K_FLOAT since a signed int32_t is not equal to an unsigned int32_t.
On top of this I added some extra bits for the it_refp check. I doubt that
those matter much but better handle all the cases.

-- 
:wq Claudio

Index: parse.c
===================================================================
RCS file: /cvs/src/usr.bin/ctfconv/parse.c,v
diff -u -p -r1.19 parse.c
--- parse.c	21 Feb 2024 13:24:37 -0000	1.19
+++ parse.c	22 Feb 2024 10:37:49 -0000
@@ -323,20 +323,30 @@ it_free(struct itype *it)
 int
 it_cmp(struct itype *a, struct itype *b)
 {
-	int diff;
-
-	if ((diff = (a->it_type - b->it_type)) != 0)
-		return diff;
-
-	/* Basic types need to have the same size. */
-	if ((a->it_type == CTF_K_INTEGER || a->it_type == CTF_K_FLOAT) &&
-	    (diff = (a->it_size - b->it_size) != 0))
-		return diff;
+	if (a->it_type > b->it_type)
+		return 1;
+	if (a->it_type < b->it_type)
+		return -1;
+
+	/* Basic types need to have the same encoding and size. */
+	if ((a->it_type == CTF_K_INTEGER || a->it_type == CTF_K_FLOAT)) {
+		if (a->it_enc > b->it_enc)
+			return 1;
+		if (a->it_enc < b->it_enc)
+			return -1;
+		if (a->it_size > b->it_size)
+			return 1;
+		if (a->it_size < b->it_size)
+			return -1;
+	}
 
 	/* Arrays need to have same number of elements */
-	if ((a->it_type == CTF_K_ARRAY) &&
-	    (diff = (a->it_nelems - b->it_nelems) != 0))
-		return diff;
+	if (a->it_type == CTF_K_ARRAY) {
+		if (a->it_nelems > b->it_nelems)
+			return 1;
+		if (a->it_nelems < b->it_nelems)
+			return -1;
+	}
 
 	/* Match by name */
 	if (!(a->it_flags & ITF_ANON) && !(b->it_flags & ITF_ANON))
@@ -349,6 +359,10 @@ it_cmp(struct itype *a, struct itype *b)
 	/* Match by reference */
 	if ((a->it_refp != NULL) && (b->it_refp != NULL))
 		return it_cmp(a->it_refp, b->it_refp);
+	if (a->it_refp == NULL)
+		return -1;
+	if (b->it_refp == NULL)
+		return 1;
 
 	return 0;
 }