4 Feb
2016
4 Feb
'16
2:17 p.m.
04.02.2016 13:39, Holger Freyther пишет:
I don't really get this - win compared to what? This is new function, not replacement
for some old code.
Yes. By the time we use n as unsigned we've explicitly checked already that it's
positive. On the other hand the diff between 2 unsigned ints can be signed.
I think __builtin_unreachable is deprecated but abort() in default: branch will do
the trick.
The name was suggested by Harald in earlier review. The string should be at least
cur_bit+1 bytes long.
So how shall it be called to make it clear that this is internal to implementation/file?
Indeed, it should be \param, or better yet - full comment instead of brief one.
We could but since it's tests I'd rather have it under VCS in bitvec_test.ok
explicitly. I think we only should ignore volatile things.
Leftover from the time I was thinking of adding tests with random values. Can be
safely removed.
cheers,
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?