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/.
pravin gerrit-no-reply at lists.osmocom.orgPatch Set 25: (22 comments) https://gerrit.osmocom.org/#/c/416/25/src/decoding.cpp File src/decoding.cpp: Line 697: LOGP(DRLCMACDL, LOGL_DEBUG, "Compress bitmap exists, " > Actually ... what do you mean by 'exists'? Is received? This means compress bitmap is present in the received bitmap. https://gerrit.osmocom.org/#/c/416/25/src/egprs_rlc_compression.cpp File src/egprs_rlc_compression.cpp: Line 22: extern const char *zero_run_len_code_list[EGPRS_CODEWORDS]; > the two arrays are not extern but defined further below. Done Line 37: /* The function expands the given tree by incorporating > Still no need to state that it is a function! For example: Done Line 40: * \param cdwd[in] Code word > plural, and indicate the length: Done Line 74: > (remove space from the blank line) Done Line 248: * \param rlen[out] Caluculate run length > fix typo. As I said before, it should say 'Calculated', not 'Calculate' (wh Done Line 267: dir = ((bmbuf[BITS_TO_BYTES(bit_pos) - 1] > Come on, in BITS_TO_BYTES you '+1' and here you '-1' again? Since this is t Done Line 268: >> (7 - MOD8(bit_pos))) & 0x01); > Drop outermost braces. Done Line 271: if (((dir&0x01) == 0) && (iter->left != NULL)) > dir is already & 0x01 above. Done Line 273: else if (((dir&0x01) == 1) && (iter->right != NULL)) > 'if (dir && (iter...' Done Line 279: (*rlen) = (iter->run_length); > drop all braces Done Line 283: /* Function to decompress compressed received block bitmap > I *see* that it is a function! Start with Done Line 285: * \param color_code_bit[in] Color code 1 for Ones runlength 0 for Zero runlength > As asked before, please explain why you call it 'color code' in the first p Done Line 297: uint8_t data = 0x0; > '= 0' ? Done Line 304: egprs_compress *compress = egprs_compress::instance(); > just ' = instance()' should suffice? Done Line 307: if (color_code_bit == 1) { > 'if (color_code_bit) {' Done Line 320: color_code_bit ? color_code_bit = 0 : color_code_bit = 1; > "color_code_bit = !color_code_bit;" Done https://gerrit.osmocom.org/#/c/416/25/src/egprs_rlc_compression.h File src/egprs_rlc_compression.h: Line 7: #define BITS_TO_BYTES(X) (((X) ? (X)/8 : 0) + 1) > Then this should be called BITPOS_TO_BYTES instead. Done Line 8: #define MOD8(X) ((X) & (0x07)) > you're using this define only once, just drop it and use '& 0x7' directly i Done Line 14: } egprs_compress_node; > What I meant: here just say "struct egprs_compress_node;", Done Line 32: { > I would move the function definition to .cpp as well Done Line 41: decode_tree_init(); > and move this one to .cpp as well. Done -- To view, visit https://gerrit.osmocom.org/416 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ieae1992ed4b02bb1e09eec2d3de1a030eabd16ce Gerrit-PatchSet: 25 Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Owner: pravin <pravin.manoharan at radisys.com> Gerrit-Reviewer: Holger Freyther <holger at freyther.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Max <msuraev at sysmocom.de> Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de> Gerrit-Reviewer: arvind.sirsikar <arvind.sirsikar at radisys.com> Gerrit-Reviewer: pravin <pravin.manoharan at radisys.com> Gerrit-HasComments: Yes