Patch Set 1:

(2 comments)

https://gerrit.osmocom.org/#/c/3991/1/src/encoding.cpp
File src/encoding.cpp:

Line 118
bitvec_set_bit(dest, H); would be more readable, don't you agree?


Line 88:        bitvec_write_field_lh(dest, &wp, 3, 2);   // "HH"
I would suggest to either use two successive "bitvec_set_bit(dest, H);" lines 
or to use something like

enum bit_value hh[] = { H, H };
bitvec_set_bits(dest, hh, ARRAY_SIZE(hh));

this avoids having to introduce a new API function to libosmocore (and related 
dependency on latest version of it) and is much more readable, IMHO.

Otherwise, great catch, thanks for the patch!


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I75dd5bebc74eea85edf9582607c774d0bba0d2a6
Gerrit-PatchSet: 1
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Owner: Minh-Quang Nguyen <[email protected]>
Gerrit-Reviewer: Harald Welte <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-HasComments: Yes

Reply via email to