Patch Set 3:

(2 comments)

https://gerrit.osmocom.org/#/c/7034/3/include/osmocom/gsm/bts_features.h
File include/osmocom/gsm/bts_features.h:

Line 29: extern const struct value_string osmo_bts_features_descs[];
It is our general policy (mentioned over and over again in patch review) to 
provide an inline function wrapper around every value_string array that is 
exported.  something like "osmo_bts_feature_name(enum osmo_bts_features 
feature)". Please add.


Line 31: static inline int osmo_bts_set_feature(struct bitvec *features, enum 
osmo_bts_features feature)
I don't think those functions are particularly performance critical / called 
frequently (and hence should be inline functions).  What was the rationale?  
I'm not fundamentally opposed, I was just wondering...


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id0c35aef11aa49aa40abe7deef1f9dbd12210776
Gerrit-PatchSet: 3
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Owner: dexter <pma...@sysmocom.de>
Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pma...@sysmocom.de>
Gerrit-HasComments: Yes

Reply via email to