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 (10 comments) (coding style comments, can't claim to have confirmed correctness with specs) https://gerrit.osmocom.org/#/c/2820/2/src/gsm/gsm0808_utils.c File src/gsm/gsm0808_utils.c: Line 149: case GSM0808_SCT_FR1: let's do it like this: bool type_extended; (i.e. not setting a value at first) switch(sc->type) { case GSM0808_SCT_FR1: case GSM0808_SCT_FR2: case GSM0808_SCT_FR3: (i.e. one combined case for all) case [...]: type_extended = false; break; case GSM0808_SCT_SCD: type_extended = true; break; default: OSMO_ASSERT(false); break; } I'd prefer actual logging instead of ASSERT(false) but it seems this file has no logging anywhere; not sure whether that is policy, so accepting that. Line 206: /* Note: If a configuration is present or not depends on the selected This first sentence has no meaning? Ah, replace "If" with "Whether", please Line 207: * codec type. If present, it can either consist of one or two octet, "octets" Line 212: break; please align the 'break' with the code, i.e. one more level of indent, to match general osmocom coding style. Line 217: case GSM0808_SCT_FR5: combine with FR4 case above Line 221: case GSM0808_SCT_HR3: combine with FR3 above Line 224: case GSM0808_SCT_HR4: again combine, same below. Better to avoid code dup, fine to give up alphabetic ordering. Line 308: * depending on the codec type */ "Whether ... octets" Line 309: switch (sc->type) { combine all similar cases, as above https://gerrit.osmocom.org/#/c/2820/2/tests/gsm0808/gsm0808_test.c File tests/gsm0808/gsm0808_test.c: Line 591: (we usually avoid double blank lines ... maybe drop one that existed before this patch) -- To view, visit https://gerrit.osmocom.org/2820 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idabb0f9620659557672e1c6b90c75481192e5c89 Gerrit-PatchSet: 2 Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Owner: dexter <pmaier at sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de> Gerrit-HasComments: Yes