> On 02 Feb 2016, at 12:19, [email protected] 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?
