Dear Andreas,
I noticed you rebased the code and I have further comments and requests for general changes.
Magic Numbers: Please avoid magic numbers where you can. There is no runtime overhead by using a define and little pre-processing overhead.
E.g. in here #define L1SAP_IS_LINK_SACCH(link_id) ((link_id & 0xC0) == 0x40)
Make your methods smaller: Please compare this with the work Daniel and me to in cleaning up your code in the osmo-pcu. By splitting up the alloc_algorithm_b code we started to notice various defects just by looking at the code.
Remove copy and paste: I think I saw the msgb in-place changing in at least two places. Avoid copy and paste (code cloning).
Avoid putting logic in deeply nested constructs:
if (a) if (b) if (c) important_stuff
kind regards holger
Holger Hans Peter Freyther wrote:
dear holger,
thanks for advices.
E.g. in here #define L1SAP_IS_LINK_SACCH(link_id) ((link_id & 0xC0) == 0x40)
i could change it (and other defines) to something like
#define GSM_LINK_ID_SACCH 0x40 #define GSM_LINK_ID_MASK 0xc0 #define L1SAP_IS_LINK_SACCH(link_id) ((link_id & GSM_LINK_ID_MASK) == GSM_LINK_ID_SACCH)
is that what you mean by avoiding magic numbers?
best regards,
andreas