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/.
Harald Welte gerrit-no-reply at lists.osmocom.orgPatch Set 1: Code-Review-1 (10 comments) https://gerrit.osmocom.org/#/c/641/1/openbsc/src/gprs/gprs_sndcp_xid.c File openbsc/src/gprs/gprs_sndcp_xid.c: Line 45: /* Encode applicable sapis (works the same in all three compression schemes) */ make sense to indicate that the caller is expected to provide a pre-allocated 'bytes' of length 2. PS1, Line 46: bytes let's call it 'out' or 'dst' instead of bytes to maeke sure it is the output Line 58: if ((nsapi < 5) || (nsapi > 15)) might make sense to state why < 5 // > 15 is not permitted. PS1, Line 92: (!(bytes))) we don't write lisp. "|| !bytes)" is sufficient ;) Line 248: if (params->max_cid > 16383) bei diesen magischen nummern waere es schoen zu wissen, wo sie herkommen. Entweder Kommentar (wenn nur einmal benutzt), oder #define mit kommentar + spec-referenz beim #define. Line 442: /* Encode data or protocol control information compression field */ In general it is good practise to always refer the specific specification chapter that defines the encoding of a message or information element above the function encoding it. Line 1132: static int decode_comp_field(const uint8_t * bytes, unsigned int bytes_len, function is long and has high indent level, should be split up Line 1352: rc = decode_comp_field(val + byte_counter, can this ever return 0 and cause an endless loop? Line 1406: struct gprs_sndcp_comp_field *comp_field; function is too long, indent level too deep. please split in multiple functions. Line 1411: LOGP(DSNDCP, LOGL_DEBUG, "SNDCP-XID:\n"); use DEBUGP when LOGP.... LOGL_DEBUG results in a longer command. -- To view, visit https://gerrit.osmocom.org/641 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If2d63fe2550864cafef3156b1dc0629037c49c1e Gerrit-PatchSet: 1 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: dexter <pmaier at sysmocom.de> Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org> Gerrit-HasComments: Yes