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/.
Holger Freyther gerrit-no-reply at lists.osmocom.org
Patch Set 8:
(11 comments)
https://gerrit.osmocom.org/#/c/416/8/src/egprs_rlc_compression.cpp
File src/egprs_rlc_compression.cpp:
Line 14: void *tall_tree_ctx;
Every allocation by talloc is a talloc context, no need to create an artificial context here. If you do it needs to be anchored with the "root" context so it shows up in leak detection.
Line 22: new_node = talloc(tall_tree_ctx, Node);
pass the parent or root as talloc context and use that and use that.. use talloc_zero.
Line 227: int search_runlen(
static and if not static make this proper doxygen comments
Line 244: /* get the bit value at the bitpos and put it in right most of dir */
wrong indent
Line 261: } /* search_runlen */
what value does this provide?
Line 263: int Decoding::decompress_crbb(
doxygen comments.. and document all variables
Line 310: } /* Decompress_CRBB */
This comment doesn't provide much info. remove it.
https://gerrit.osmocom.org/#/c/416/8/src/pcu_main.cpp
File src/pcu_main.cpp:
Line 172: tall_tree_ctx = talloc_named_const(tall_pcu_ctx, 0, "decode-tree context");
nack.
https://gerrit.osmocom.org/#/c/416/8/tests/tbf/TbfTest.cpp
File tests/tbf/TbfTest.cpp:
Line 43: #define NUMBER_OF_TEST_CASE 9
Move T4 testcases into a dedicated t4 test directory/application.
Line 57: }test[NUMBER_OF_TEST_CASE] = { { (int8_t)67, (uint8_t)1, {0x02, 0x0c, 0xa0, 0x30, 0xcb, 0x1a, 0x0c, 0xe3, 0x6c},
Let the compiler count it for you. If you want to make sure it is 9.. then use a static assert comparing the array size with your magic number
Line 1624: tall_tree_ctx = talloc_named_const(tall_pcu_ctx, 0, "decode-tree context");
How many other talloc users do you see that do that? It could be a hint that this is not the correct way?
--
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: 8
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: pravin <pravin.manoharan at radisys.com>
Gerrit-HasComments: Yes