Apply Michel's tidy-ups

src/bitset.h

@@ -7,43 +7,66 @@
 #include <string.h>
 #include <pthread.h>
 
+/*
+ * Make the bitfield words 'opaque' to prevent code
+ * poking at the bits directly without using these
+ * accessors/macros
+ */
+typedef uint64_t	bitfield_word_t;
+typedef bitfield_word_t * bitfield_p;
 
-static inline char char_with_bit_set(uint64_t num) { return 1<<(num%8); }
+#define BITFIELD_WORD_SIZE	sizeof(bitfield_word_t)
+#define BITS_PER_WORD	(BITFIELD_WORD_SIZE * 8)
+
+#define BIT_MASK(_idx)	\
+			(1LL << ((_idx) & (BITS_PER_WORD - 1)))
+#define BIT_WORD(_b, _idx)	\
+			((bitfield_word_t*)(_b))[(_idx) / BITS_PER_WORD]
+
+/* Calculates the number of words needed to store _bytes number of bytes
+ * this is added to accommodate code that wants to use bytes sizes
+ */
+#define BIT_WORDS_FOR_SIZE(_bytes) \
+			(((_bytes) + (BITFIELD_WORD_SIZE -1)) & ~(BITFIELD_WORD_SIZE))
+#define BIT_WORDS_FOR_COUNT(_bits) \
+			(((_bits) + (BITS_PER_WORD -1)) / (BITFIELD_WORD_SIZE))
+
+/** Return the bit value ''idx'' in array ''b'' */
+static inline int bit_get(bitfield_p b, uint64_t idx) {
+	return (BIT_WORD(b, idx) >> (idx & (BITS_PER_WORD-1))) & 1;
+}
 
 /** Return 1 if the bit at ''idx'' in array ''b'' is set */
-static inline int bit_is_set(char* b, uint64_t idx) {
-	return (b[idx/8] & char_with_bit_set(idx)) != 0;
+static inline int bit_is_set(bitfield_p b, uint64_t idx) {
+	return bit_get(b, idx);
 }
 /** Return 1 if the bit at ''idx'' in array ''b'' is clear */
-static inline int bit_is_clear(char* b, uint64_t idx) {
-	return !bit_is_set(b, idx);
+static inline int bit_is_clear(bitfield_p b, uint64_t idx) {
+	return !bit_get(b, idx);
 }
 /** Tests whether the bit at ''idx'' in array ''b'' has value ''value'' */
-static inline int bit_has_value(char* b, uint64_t idx, int value) {
-	if (value) { return bit_is_set(b, idx); }
-	else { return bit_is_clear(b, idx); }
+static inline int bit_has_value(bitfield_p b, uint64_t idx, int value) {
+	return bit_get(b, idx) == !!value;
 }
 /** Sets the bit ''idx'' in array ''b'' */
-static inline void bit_set(char* b, uint64_t idx) {
-	b[idx/8] |= char_with_bit_set(idx);
-	//__sync_fetch_and_or(b+(idx/8), char_with_bit_set(idx));
+static inline void bit_set(bitfield_p b, uint64_t idx) {
+	BIT_WORD(b, idx) |= BIT_MASK(idx);
 }
 /** Clears the bit ''idx'' in array ''b'' */
-static inline void bit_clear(char* b, uint64_t idx) {
-	b[idx/8] &= ~char_with_bit_set(idx);
-	//__sync_fetch_and_nand(b+(idx/8), char_with_bit_set(idx));
+static inline void bit_clear(bitfield_p b, uint64_t idx) {
+	BIT_WORD(b, idx) &= ~BIT_MASK(idx);
 }
 /** Sets ''len'' bits in array ''b'' starting at offset ''from'' */
-static inline void bit_set_range(char* b, uint64_t from, uint64_t len)
+static inline void bit_set_range(bitfield_p b, uint64_t from, uint64_t len)
 {
-	for ( ; from%8 != 0 && len > 0 ; len-- ) {
+	for ( ; (from % BITS_PER_WORD) != 0 && len > 0 ; len-- ) {
 		bit_set( b, from++ );
 	}
 
-	if (len >= 8) {
-		memset(b+(from/8), 255, len/8 );
+	if (len >= BITS_PER_WORD) {
+		memset(&BIT_WORD(b, from), 0xff, len / 8 );
 		from += len;
-		len = (len%8);
+		len = len % BITS_PER_WORD;
 		from -= len;
 	}
 
@@ -52,16 +75,16 @@ static inline void bit_set_range(char* b, uint64_t from, uint64_t len)
 	}
 }
 /** Clears ''len'' bits in array ''b'' starting at offset ''from'' */
-static inline void bit_clear_range(char* b, uint64_t from, uint64_t len)
+static inline void bit_clear_range(bitfield_p b, uint64_t from, uint64_t len)
 {
-	for ( ; from%8 != 0 && len > 0 ; len-- ) {
+	for ( ; (from % BITS_PER_WORD) != 0 && len > 0 ; len-- ) {
 		bit_clear( b, from++ );
 	}
 
-	if (len >= 8) {
-		memset(b+(from/8), 0, len/8 );
+	if (len >= BITS_PER_WORD) {
+		memset(&BIT_WORD(b, from), 0, len / 8 );
 		from += len;
-		len = (len%8);
+		len = len % BITS_PER_WORD;
 		from -= len;
 	}
 
@@ -75,16 +98,16 @@ static inline void bit_clear_range(char* b, uint64_t from, uint64_t len)
   * bits that are the same as the first one specified. If ''run_is_set'' is
   * non-NULL, the value of that bit is placed into it.
   */
-static inline uint64_t bit_run_count(char* b, uint64_t from, uint64_t len, int *run_is_set) {
-	uint64_t* current_block;
+static inline uint64_t bit_run_count(bitfield_p b, uint64_t from, uint64_t len, int *run_is_set) {
 	uint64_t count = 0;
-	int first_value = bit_is_set(b, from);
+	int first_value = bit_get(b, from);
+	bitfield_word_t word_match = first_value ? -1 : 0;
 
 	if ( run_is_set != NULL ) {
 		*run_is_set = first_value;
 	}
 
-	for ( ; (from+count) % 64 != 0 && len > 0; len--) {
+	for ( ; ((from + count) % BITS_PER_WORD) != 0 && len > 0; len--) {
 		if (bit_has_value(b, from + count, first_value)) {
 			count++;
 		} else {
@@ -92,10 +115,9 @@ static inline uint64_t bit_run_count(char* b, uint64_t from, uint64_t len, int *
 		}
 	}
 
-	for ( ; len >= 64 ; len -= 64 ) {
-		current_block = (uint64_t*) (b + ((from+count)/8));
-		if (*current_block == ( first_value ? UINT64_MAX : 0 ) ) {
-			count += 64;
+	for ( ; len >= BITS_PER_WORD ; len -= BITS_PER_WORD ) {
+		if (BIT_WORD(b, from + count) == word_match) {
+			count += BITS_PER_WORD;
 		} else {
 			break;
 		}
@@ -154,7 +176,7 @@ struct bitset {
 	int resolution;
 	struct bitset_stream *stream;
 	int stream_enabled;
-	char bits[];
+	bitfield_word_t bits[];
 };
 
 /** Allocate a bitset for a file of the given size, and chunks of the
@@ -162,9 +184,12 @@ struct bitset {
   */
 static inline struct bitset *bitset_alloc( uint64_t size, int resolution )
 {
-	struct bitset *bitset = xmalloc(
-		sizeof( struct bitset ) + ( size + resolution - 1 ) / resolution
-	);
+	// calculate a size to allocate that is a multiple of the size of the
+	// bitfield word
+	size_t bitfield_size =
+			BIT_WORDS_FOR_SIZE((( size + resolution - 1 ) / resolution));
+	struct bitset *bitset = xmalloc(sizeof( struct bitset ) + bitfield_size);
+
 	bitset->size = size;
 	bitset->resolution = resolution;
 	/* don't actually need to call pthread_mutex_destroy '*/

src/mirror.c

@@ -636,7 +636,7 @@ static void mirror_read_cb( struct ev_loop *loop, ev_io *w, int revents )
 	return;
 }
 
-void mirror_timeout_cb( struct ev_loop *loop, ev_timer *w __attribute__((unused)), int revents )
+static void mirror_timeout_cb( struct ev_loop *loop, ev_timer *w __attribute__((unused)), int revents )
 {
 	if ( !(revents & EV_TIMER ) ) {
 		warn( "Mirror timeout called but no timer event signalled" );
@@ -648,7 +648,7 @@ void mirror_timeout_cb( struct ev_loop *loop, ev_timer *w __attribute__((unused)
 	return;
 }
 
-void mirror_abandon_cb( struct ev_loop *loop, ev_io *w, int revents )
+static void mirror_abandon_cb( struct ev_loop *loop, ev_io *w, int revents )
 {
 	struct mirror_ctrl* ctrl = (struct mirror_ctrl*) w->data;
 	NULLCHECK( ctrl );
@@ -666,7 +666,7 @@ void mirror_abandon_cb( struct ev_loop *loop, ev_io *w, int revents )
 }
 
 
-void mirror_limit_cb( struct ev_loop *loop, ev_timer *w, int revents )
+static void mirror_limit_cb( struct ev_loop *loop, ev_timer *w, int revents )
 {
 	struct mirror_ctrl* ctrl = (struct mirror_ctrl*) w->data;
 	NULLCHECK( ctrl );
@@ -694,7 +694,7 @@ void mirror_limit_cb( struct ev_loop *loop, ev_timer *w, int revents )
  * if it has, start migrating. If it's not finished, then enabling the bitset
  * stream does not go well for us.
  */
-void mirror_begin_cb( struct ev_loop *loop, ev_timer *w, int revents )
+static void mirror_begin_cb( struct ev_loop *loop, ev_timer *w, int revents )
 {
 	struct mirror_ctrl* ctrl = (struct mirror_ctrl*) w->data;
 	NULLCHECK( ctrl );

src/readwrite.c

@@ -105,8 +105,9 @@ void fill_request(struct nbd_request *request, int type, off64_t from, int len)
 {
 	request->magic  = htobe32(REQUEST_MAGIC);
 	request->type   = htobe32(type);
-	((int*) request->handle)[0] = rand();
-	((int*) request->handle)[1] = rand();
+	uint32_t * randa = (uint32_t*)request->handle;
+	randa[0] = rand();
+	randa[1] = rand();
 	request->from   = htobe64(from);
 	request->len    = htobe32(len);
 }

src/util.h

@@ -116,6 +116,7 @@ uint64_t monotonic_time_ms(void);
 #define fatal(msg, ...) do { \
 	myloglev(4, msg, ##__VA_ARGS__); \
 	error_handler(1); \
+	exit(1); /* never-reached, this is to make static code analizer happy */ \
 } while(0)
 
 

tests/unit/check_bitset.c

@@ -10,7 +10,7 @@
 START_TEST(test_bit_set)
 {
 	uint64_t num = 0;
-	char *bits = (char*) #
+	bitfield_p bits = (bitfield_p) #
 
 #define TEST_BIT_SET(bit, newvalue) \
 	bit_set(bits, (bit)); \
@@ -27,7 +27,7 @@ END_TEST
 START_TEST(test_bit_clear)
 {
 	uint64_t num = 0xffffffffffffffff;
-	char *bits = (char*) #
+	bitfield_p bits = (bitfield_p) #
 
 #define TEST_BIT_CLEAR(bit, newvalue) \
 	bit_clear(bits, (bit)); \
@@ -44,7 +44,7 @@ END_TEST
 START_TEST(test_bit_tests)
 {
 	uint64_t num = 0x5555555555555555;
-	char *bits = (char*) #
+	bitfield_p bits = (bitfield_p) #
 
 	fail_unless(bit_has_value(bits, 0, 1), "bit_has_value malfunction");
 	fail_unless(bit_has_value(bits, 1, 0), "bit_has_value malfunction");
@@ -58,7 +58,7 @@ END_TEST
 
 START_TEST(test_bit_ranges)
 {
-	char buffer[4160];
+	bitfield_word_t buffer[BIT_WORDS_FOR_SIZE(4160)];
 	uint64_t  *longs = (unsigned long*) buffer;
 	uint64_t i;
 
@@ -84,7 +84,7 @@ END_TEST
 
 START_TEST(test_bit_runs)
 {
-	char buffer[256];
+	bitfield_word_t buffer[BIT_WORDS_FOR_SIZE(256)];
 	int i, ptr=0, runs[] = {
 		56,97,22,12,83,1,45,80,85,51,64,40,63,67,75,64,94,81,79,62
 	};

tests/unit/check_serve.c

@@ -93,7 +93,7 @@ END_TEST
 
 int connect_client( char *addr, int actual_port, char *source_addr )
 {
-	int client_fd;
+	int client_fd = -1;
 
 	struct addrinfo hint;
 	struct addrinfo *ailist, *aip;