Check if a value will fit in bitfield before setting.

3 files changed, 26 insertions, 14 deletions

diff --git a/src/bitfield/8byte.c b/src/bitfield/8byte.c
index 77251996..3b6b5466 100644
--- a/src/bitfield/8byte.c
+++ b/src/bitfield/8byte.c
@@ -40,16 +40,17 @@ uint64_t get_bit_field(uint64_t source, const uint16_t startBit,
     return int_result;
 }
 
-/**
- * TODO it would be nice to have a warning if you call with this a value that
- * won't fit in the number of bits you've specified it should use.
- */
-void set_bit_field(uint64_t* destination, uint64_t value, const uint16_t offset,
+bool set_bit_field(uint64_t* destination, uint64_t value, const uint16_t offset,
         const uint16_t bit_count) {
+    if(value > bitmask(bit_count)) {
+        return false;
+    }
+
     int shiftDistance = EIGHTBYTE_BIT - offset - bit_count;
     value <<= shiftDistance;
     *destination &= ~(bitmask(bit_count) << shiftDistance);
     *destination |= value;
+    return true;
 }
 
 uint8_t nth_byte(const uint64_t source, const uint16_t byte_index) {
diff --git a/src/bitfield/8byte.h b/src/bitfield/8byte.h
index 9cbf61a9..36b5fe60 100644
--- a/src/bitfield/8byte.h
+++ b/src/bitfield/8byte.h
@@ -55,8 +55,11 @@ uint64_t get_bit_field(uint64_t source, const uint16_t offset,
  * value - the value to set in the bit field.
  * offset - the starting index of the bit field (beginning from 0).
  * bit_count - the number of bits to set in the data.
+ *
+ * Returns true if the bit_count is enough to fully represent the value, and
+ *      false if it will not fit.
  */
-void set_bit_field(uint64_t* destination, uint64_t value, const uint16_t offset,
+bool set_bit_field(uint64_t* destination, uint64_t value, const uint16_t offset,
         const uint16_t bit_count);
 
 /* Public: Retreive the nth byte out of 8 bytes in a uint64_t.
diff --git a/tests/8byte_tests.c b/tests/8byte_tests.c
index f4188bfe..64c1a399 100644
--- a/tests/8byte_tests.c
+++ b/tests/8byte_tests.c
@@ -103,19 +103,26 @@ START_TEST (test_get_off_byte_boundary)
     ck_assert_int_eq(result, 0x01);
 } END_TEST
 
+START_TEST (test_set_wont_fit)
+{
+    uint64_t data = 0;
+    fail_if(set_bit_field(&data, 100, 0, 1));
+}
+END_TEST
+
 START_TEST (test_set_field)
 {
     uint64_t data = 0;
-    set_bit_field(&data, 1, 0, 1);
+    fail_unless(set_bit_field(&data, 1, 0, 1));
     uint64_t result = get_bit_field(data, 0, 1, false);
     ck_assert_int_eq(result, 0x1);
     data = 0;
-    set_bit_field(&data, 1, 1, 1);
+    fail_unless(set_bit_field(&data, 1, 1, 1));
     result = get_bit_field(data, 1, 1, false);
     ck_assert_int_eq(result, 0x1);
 
     data = 0;
-    set_bit_field(&data, 0xf, 3, 4);
+    fail_unless(set_bit_field(&data, 0xf, 3, 4));
     result = get_bit_field(data, 3, 4, false);
     ck_assert_int_eq(result, 0xf);
 }
@@ -124,14 +131,14 @@ END_TEST
 START_TEST (test_set_doesnt_clobber_existing_data)
 {
     uint64_t data = 0xFFFC4DF300000000;
-    set_bit_field(&data, 0x4fc8, 16, 16);
+    fail_unless(set_bit_field(&data, 0x4fc8, 16, 16));
     uint64_t result = get_bit_field(data, 16, 16, false);
     fail_unless(result == 0x4fc8,
             "Field retrieved in 0x%llx was 0x%llx instead of 0x%x", data, result,
             0xc84f);
 
     data = 0x8000000000000000;
-    set_bit_field(&data, 1, 21, 1);
+    fail_unless(set_bit_field(&data, 1, 21, 1));
     fail_unless(data == 0x8000040000000000LLU,
             "Expected combined value 0x8000040000000000 but got 0x%llx%llx",
             data >> 32, data);
@@ -141,7 +148,7 @@ END_TEST
 START_TEST (test_set_off_byte_boundary)
 {
     uint64_t data = 0xFFFC4DF300000000;
-    set_bit_field(&data, 0x12, 12, 8);
+    fail_unless(set_bit_field(&data, 0x12, 12, 8));
     uint64_t result = get_bit_field(data, 12, 12, false);
     ck_assert_int_eq(result,0x12d);
 }
@@ -150,14 +157,14 @@ END_TEST
 START_TEST (test_set_odd_number_of_bits)
 {
     uint64_t data = 0xFFFC4DF300000000LLU;
-    set_bit_field(&data, 0x12, 11, 5);
+    fail_unless(set_bit_field(&data, 0x12, 11, 5));
     uint64_t result = get_bit_field(data, 11, 5, false);
     fail_unless(result == 0x12,
             "Field set in 0x%llx%llx%llx%llx was 0x%llx instead of 0x%llx", data, result,
             0x12);
 
     data = 0xFFFC4DF300000000LLU;
-    set_bit_field(&data, 0x2, 11, 5);
+    fail_unless(set_bit_field(&data, 0x2, 11, 5));
     result = get_bit_field(data, 11, 5, false);
     fail_unless(result == 0x2,
             "Field set in 0x%llx%llx%llx%llx was 0x%llx instead of 0x%llx", data, result,
@@ -202,6 +209,7 @@ Suite* bitfieldSuite(void) {
     tcase_add_test(tc_core, test_multi_byte);
     tcase_add_test(tc_core, test_get_multi_byte);
     tcase_add_test(tc_core, test_get_off_byte_boundary);
+    tcase_add_test(tc_core, test_set_wont_fit);
     tcase_add_test(tc_core, test_set_field);
     tcase_add_test(tc_core, test_set_doesnt_clobber_existing_data);
     tcase_add_test(tc_core, test_set_off_byte_boundary);