This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
Neels Hofmeyr gerrit-no-reply at lists.osmocom.orgPatch Set 2: Code-Review-1 (9 comments) A general problem is that the split of Algorithm B is super complex to review, yet this patch places more cosmetic changes on top -- tldr syndrome. I accept that there is complex logic merely spread across functions to clarify, and now that I read these functions on their own I spot various little "API cracks"; i.e. that I am stricter on the code now that it is split into a smaller function, and would not have dared to touch the complexity otherwise. That's why I'm stopping the review halfway through: I fear that I'm annoying more than helping and using our time for no benefit. It would be easier to review this in smaller chunks :) https://gerrit.osmocom.org/#/c/3760/2/src/bts.h File src/bts.h: Line 210 it would be nice to have a brief explanation in the commit log why this cust thing can be dropped. https://gerrit.osmocom.org/#/c/3760/2/src/gprs_rlcmac_ts_alloc.cpp File src/gprs_rlcmac_ts_alloc.cpp: Line 390: if (trx) { Does this function make sense with trx == NULL? If yes, it needs to be documented above. If not, drop the check. Line 848: /*! Assign fiven UL timeslots to UL TBF "five"? "given"? :) Line 872: /*! Assign fiven DL timeslots to DL TBF same Line 892: /*! Update timeslot counters "Count used bits in slots and reserved_slots bitmasks"? It doesn't really update anything, rather returns a count. Line 912: * \returns UL or DL timeslot number and document what is returned in *ts Line 914: static uint8_t get_slots_single(gprs_rlcmac_trx *trx, const gprs_rlcmac_tbf *tbf, uint8_t dl_slots, uint8_t ul_slots, the naming of "get_slots" + "single" cracks my head. Get several slots or a single one? Line 923: return pcu_lsb(ret); should *ts be assigned a value when we return here? If not, this special case needs documenting. Line 925: return ret & (1 << (*ts)); It seems that we quite possibly return no TS number: if the dl_slots & ul_slots mask results in a bitmask that doesn't match 1 << *ts. Is that expected? Document it? -- To view, visit https://gerrit.osmocom.org/3760 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I02da2b8ba8c9c8815dae0e39e1fed277ca0df171 Gerrit-PatchSet: 2 Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Owner: Max <msuraev at sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de> Gerrit-HasComments: Yes