On 02 Feb 2016, at 12:19, suraev@alumni.ntnu.no wrote:
Dear Max,
- memcpy(tmp, bv->data, 2);
- return osmo_load16be(tmp) >> (16 - num_bits);
load16be is working byte by byte as well so what do we win by this load?
+/*! \brief fill num_bits with \fill starting from the current position
- returns 0 on success, negative otherwise (out of vector boundary)
- */
+int bitvec_fill(struct bitvec *bv, unsigned int num_bits, enum bit_value fill) +{
- unsigned i, stop = bv->cur_bit + num_bits;
- for (i = bv->cur_bit; i < stop; i++)
if (bitvec_set_bit(bv, fill) < 0)return -EINVAL;- return 0;
+}
/*! \brief pad all remaining bits up to num_bits */ int bitvec_spare_padding(struct bitvec *bv, unsigned int up_to_bit) {
- unsigned int i;
- for (i = bv->cur_bit; i <= up_to_bit; i++)
bitvec_set_bit(bv, L);
- int n = up_to_bit - bv->cur_bit + 1;
- if (n < 1)
return 0;
so we are going from unsigned to int and then 'n' is "converted" to unsigned int as well. Is this what we want?
+/*! \brief convert enum to corresponding character */ +char bit_value_to_char(enum bit_value v) +{
- switch (v) {
- case ZERO: return '0';
- case ONE: return '1';
- case L: return 'L';
- case H: return 'H';
- }
- /* make compiler happy - "avoid control reaches end of non-void function" warning: */
seeing is believing? Which gcc? I had proposed to use __builtin_unreachable here. If we know we don't end up there, put this into the "branch" and the warning is gone. Or abort() or __builtin_trap.
- return '?';
+}
+/*! \brief prints bit vector to provided string
- It's caller's responcibility to ensure that we won't shoot him in the foot.
typo, how long is "str"? So either pass a size or write it in the comment of how long it needs to be depending on cur_bit? And what does the _r stand for? Is it similiar to the libc _r functions where an external buffer/pointer is provided?
+/* we assume that x have at least 1 non-b bit */ +static inline unsigned _leading_bits(uint8_t x, bool b)
_ and __ are reserved for the system, let us not use it. By having this method as static we already don't pollute the global namespace.
+/*! \brief Return number (bits) of uninterrupted run of \b in \bv starting from the MSB */
Are you sure that \b and \bv refer to the params? not like \param b, \param bv? Did you look at the documentation generated by doxygen?
fprintf(stderr, "out: %s\n", osmo_hexdump(out, sizeof(out)));
printf("out: %s\n", osmo_hexdump(out, sizeof(out)));
why? more output to be tracked? we could not ignore stderr if we want to?
OSMO_ASSERT(out[0] == 0xff); OSMO_ASSERT(out[in_size+1] == 0xff);@@ -72,11 +145,77 @@ static void test_unhex(const char *hex)
int main(int argc, char **argv) {
- srand(time(NULL));
why?