Patch Set 1:

(6 comments)

I'm not certain enough to + or -

https://gerrit.osmocom.org/#/c/4337/1/include/osmocom/core/bitvec.h
File include/osmocom/core/bitvec.h:

Line 61: int bitvec_set_u64(struct bitvec *bv, uint64_t v, uint8_t num_bits, 
bool use_lh);
should this be accompanied by a bitvec_get_u64()?


Line 62: int bitvec_set_uint(struct bitvec *bv, unsigned int in, unsigned int 
count);
we could deprecate it to avoid a function call to bitvec_set_u64() ... or just 
leave it to the compiler's optimization.


https://gerrit.osmocom.org/#/c/4337/1/src/bitvec.c
File src/bitvec.c:

Line 218: /*! set multiple bits (based on numeric value) at current pos
(while at it, let's also add a '.' to end the sentence; best also add '.' at 
the end of each \param and \returns.)


Line 224: int bitvec_set_u64(struct bitvec *bv, uint64_t v, uint8_t num_bits, 
bool use_lh)
this would be the first implementation to add a use_lh semantic. Why is it 
needed / why don't the others need it? How did adding an int with L/H work 
before this? Would it make sense to add such a flag in struct bitvec?


Line 235:               if (v & ((uint64_t)1 << (num_bits - i - 1)))
'1LL' instead of cast?


Line 246: /*! set multiple bits (based on numeric value) at current pos
('.' plz)


-- 
To view, visit https://gerrit.osmocom.org/4337
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1b670dacb55fb3063271d045f9faa10fccba10a6
Gerrit-PatchSet: 1
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Owner: Max <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Minh-Quang Nguyen <[email protected]>
Gerrit-Reviewer: Neels Hofmeyr <[email protected]>
Gerrit-HasComments: Yes

Reply via email to