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 1: Code-Review-1 (8 comments) https://gerrit.osmocom.org/#/c/702/1//COMMIT_MSG Commit Message: Line 7: Describe the issue with EGPRS PUAN encoding This commit adds a test case, so the summary should be something like "add test case for ..." Line 17: to be corrected. Sorry, I don't follow at all. * What are you trying to say about the hex values (and why post the same hex values twice)? * What do you mean by "Later"? * What do you mean by "frame work", maybe name a specific test instead? * What exactly is the issue with OTA, maybe add a link to a mailing list posting? Also see inline about adding a wrong assertion, which is my main reason for rejecting. (Though I must admit I don't fully understand the issue this is testing for, the assertion must test for the right behavior.) https://gerrit.osmocom.org/#/c/702/1/tests/tbf/TbfTest.cpp File tests/tbf/TbfTest.cpp: Line 663: Exist_Multislot_capability = 1; (strictly speaking, there is an indent too many before "Exist_") Line 683: OSMO_ASSERT(ul_tbf != NULL); Rather use OSMO_ASSERT(ul_tbf) (drop != NULL). Out of curiosity: check_tbf() below also does the OSMO_ASSERT(ul_tbf), any reason to not use it here as well? Line 696: 0x00 | 0xf << 2, /* GPRS_RLCMAC_DATA_BLOCK << 6, CV = 15 */ Why "0x00 |" ? 0x00 | foo == foo Line 697: uint8_t(0 | (tfi << 1)), 0 | foo == foo? Line 698: uint8_t(1), /* BSN:7, E:1 */ no need to cast to uint8_t, right? Line 753: OSMO_ASSERT(!memcmp(msg2->data, msg1->data, msg1->data_len)); This makes no sense. If this assert is wrong, then this test confirms wrong behavior. Fix the issue and commit the test with the right assertion at the same time. -- To view, visit https://gerrit.osmocom.org/702 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I00662a564f64c0c83627401ae8f7bfef0f0a5de8 Gerrit-PatchSet: 1 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: Holger Freyther <holger at freyther.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de> Gerrit-Reviewer: arvind.sirsikar <arvind.sirsikar at radisys.com> Gerrit-HasComments: Yes