Hi List and Lynxis, I've been reading through the ns2 code more thoroughly and had some thoughts for improvement. As we have no users yet, and the code is unreleased, we can still make changes now. == unused argument in ns2_recv_vc int ns2_recv_vc(struct gprs_ns2_inst *nsi, struct gprs_ns2_vc *nsvc, struct msgb *msg) The 'nsi' is not used and should be redundant, as the nsvc has a back-pointer anyway, right? == consider using an osmo_ prefix to all symbols / types The fact that the old code doesn't have that is a tribute to its age, and not something we need to keep. The current code has quite a bit of 'gprs_ns2' prefixing for types, but not for the symbols/functions. At least that inconsistency should be resolved, so all have the same prefix, even if it is without osmo. == const-ify input arguments If arguments pointing to data are used read-only, let's express that by the const qualifier. The easy ones are the likes of "gprs_ns2_is_frgre_bind(struct gprs_ns2_vc_bind *bind)", but there are definitely many more. == use of msgb->dst In several other osmocom implementations we use msgb->dst to point to the underlying element receiving or transmitting a message. So I could imagine ns2_recv_vc() not only without the 'nsi' argument, but also without the 'nsvc' argument, if we introduce the convention that msgb->dst would always point to the ns-vc. Not sure here, it has pros and cons. Just brainstorming. == output arguments vs. return value There are functions like gprs_ns2_find_vc_by_sockaddr() where the result is not returned, but rather a **pointer output argument is used. What is the rationale here? I don't understand what benefit the extra 0/1 return value gives. The only other return value is -EINVAL if the **ptr was NULL - an error situation that wouldn't exist if we simply returned the pointer from the function. Regards, Harald -- - Harald Welte <laforge(a)osmocom.org> http://laforge.gnumonks.org/ ============================================================================ "Privacy in residential applications is a desirable marketing option." (ETSI EN 300 175-7 Ch. A6)