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 3: I can only reiterate my previous comment: In short: set to INVALID only where it is supposed to be INVALID, and/or, input-check on that called function that potentially receives the INVALID items. It is currently unclear whether the code will happily use INVALID items as if they were valid. That should be easy to see. In long: " My point remains that in gprs_rlc_mcs_cps() the punct/punct2 are used in calculations directly without checking against INVALID items. By initializing both punct values to "INVALID" in create_dl_acked_block(), it is suggested that INVALID values may be passed on to gprs_rlc_mcs_cps() -- if that weren't the case, it would not be necessary to initialize to INVALID. So, gprs_rlc_mcs_cps() must check that it isn't using INVALID enum values in its bitfield calculations. Would it make more sense to set punct2 to INVALID explicitly in specific cases? Even though incidentally, the cases where one (or both??) args are INVALID might match the internal expectations of the switch() statement, this is bad coding style. Each function's API should make sense on its own. " Let's stop discussing and see another patch set... In case your intention is to abandon this, please clearly say so; but I assume it's not much effort and would appreciate if you could follow up. -- To view, visit https://gerrit.osmocom.org/1411 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ice54edc7e4a936eb2f2dd8a243673a30dceef542 Gerrit-PatchSet: 3 Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Owner: arvind.sirsikar <arvind.sirsikar at radisys.com> Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de> Gerrit-Reviewer: arvind.sirsikar <arvind.sirsikar at radisys.com> Gerrit-HasComments: No