From 77f333423b944dbd8b87fcf45db6be4f917e2470 Mon Sep 17 00:00:00 2001 From: Alex Young Date: Wed, 26 Feb 2014 15:19:03 +0000 Subject: [PATCH] Apply Michel's tidy-ups --- src/bitset.h | 101 ++++++++++++++++++++++++-------------- src/mirror.c | 8 +-- src/readwrite.c | 5 +- src/util.h | 1 + tests/unit/check_bitset.c | 10 ++-- tests/unit/check_serve.c | 2 +- 6 files changed, 77 insertions(+), 50 deletions(-) diff --git a/src/bitset.h b/src/bitset.h index 2a3ef39..cd0927d 100644 --- a/src/bitset.h +++ b/src/bitset.h @@ -7,43 +7,66 @@ #include #include +/* + * 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,34 +98,33 @@ 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--) { - if (bit_has_value(b, from+count, first_value)) { + for ( ; ((from + count) % BITS_PER_WORD) != 0 && len > 0; len--) { + if (bit_has_value(b, from + count, first_value)) { count++; } else { return count; } } - 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; } } for ( ; len > 0; len-- ) { - if ( bit_has_value(b, from+count, first_value) ) { + if ( bit_has_value(b, from + count, first_value) ) { count++; } } @@ -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 '*/ diff --git a/src/mirror.c b/src/mirror.c index a53b83c..776c345 100644 --- a/src/mirror.c +++ b/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 ); diff --git a/src/readwrite.c b/src/readwrite.c index 731e5ca..bb348d3 100644 --- a/src/readwrite.c +++ b/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); } diff --git a/src/util.h b/src/util.h index 90abf21..cb20bca 100644 --- a/src/util.h +++ b/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) diff --git a/tests/unit/check_bitset.c b/tests/unit/check_bitset.c index d334347..1dda229 100644 --- a/tests/unit/check_bitset.c +++ b/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 }; diff --git a/tests/unit/check_serve.c b/tests/unit/check_serve.c index aff7cd3..b00bb1a 100644 --- a/tests/unit/check_serve.c +++ b/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;