[libdispatch-dev] PATCH - bugfix for dispatch_priority.c spurious failures where actual blocks completed != expected

Joakim Johansson jocke at tbricks.com
Wed Jul 28 08:40:22 PDT 2010


I’ve had some problems with spurious failures of dispatch_priority/dispatch_priority2 tests on Solaris.

It actually turned out to be a small bug in the test: ‘iterdone’ being size_t would wrap around rather than being negative 
(as it seems was assumed in the code). 

It could be fixed by changing the condition to look from the wraparound, or alternatively by consistently changing the use of size_t for the various ‘counts’ to use the long type instead.

I’ve attached both flavors of the fix for consideration, both variants works fine now without spurious failures.

Cheers,

Joakim

First patch variant checking for wraparound:
---------------------------------

octo.tbricks.com:gcd/trunk/testing> svn diff dispatch_priority.c
Index: dispatch_priority.c
===================================================================
--- dispatch_priority.c	(revision 188)
+++ dispatch_priority.c	(working copy)
@@ -100,8 +100,8 @@
 		histogram();
 		test_stop();
 		exit(0);
-	} else if (iterdone > 0) {
-		__sync_add_and_fetch(count, 1);
+	} else if ((iterdone > 0) && (iterdone <= ITERATIONS)) { // We need to check that iterdone is not > ITERATIONS, as size_t is signed
+		__sync_add_and_fetch(count, 1);                      // 'iterations' wraps around and some thread may otherwise increase 'count' when the test is done 
 	}
 }

————————————————

Alternative patch changing the type from size_t to long

————————————————


octo.tbricks.com:gcd/trunk/testing> svn diff dispatch_priority.c
Index: dispatch_priority.c
===================================================================
--- dispatch_priority.c	(revision 188)
+++ dispatch_priority.c	(working copy)
@@ -48,21 +48,21 @@
 int priorities[PRIORITIES] = { DISPATCH_QUEUE_PRIORITY_LOW, DISPATCH_QUEUE_PRIORITY_DEFAULT, DISPATCH_QUEUE_PRIORITY_HIGH };
 
 union {
-	size_t count;
+	long count;
 	char padding[64];
 } counts[PRIORITIES];
 
-#define ITERATIONS (size_t)(PRIORITIES * BLOCKS * 0.50)
-size_t iterations = ITERATIONS;
+#define ITERATIONS (long)(PRIORITIES * BLOCKS * 0.50)
+long iterations = ITERATIONS;
 
 void
 histogram(void) {
-	size_t maxcount = BLOCKS;
-	size_t sc[PRIORITIES];
+	long maxcount = BLOCKS;
+	long sc[PRIORITIES];
 	
-	size_t total = 0;
+	long total = 0;
 	
-	size_t x,y;
+	long x,y;
 	for (y = 0; y < PRIORITIES; ++y) {
 		sc[y] = counts[y].count;
 	}
@@ -86,10 +86,10 @@
 void
 cpubusy(void* context)
 {
-	size_t *count = context;
-	size_t iterdone;
+	long *count = context;
+	long iterdone;
 
-	size_t idx;
+	long idx;
 	for (idx = 0; idx < LOOP_COUNT; ++idx) {
 		if (done) break;
 	}
octo.tbricks.com:gcd/trunk/testing> 





More information about the libdispatch-dev mailing list