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.org
Patch Set 4: Code-Review+2
(3 comments)
https://gerrit.osmocom.org/#/c/3194/4/src/libvlr/vlr_access_req_fsm.c
File src/libvlr/vlr_access_req_fsm.c:
Line 416: if (res) {
we have an OSMO_ASSERT(res) above, and then a runtime if (res) here. Either of the two doesn't make sense. We can either ASSERT and remove the if, or we can kep he if (but then remove the *res from LOGPFSM outside of the if(res) clause.
https://gerrit.osmocom.org/#/c/3194/4/src/libvlr/vlr_auth_fsm.c
File src/libvlr/vlr_auth_fsm.c:
Line 552: .name = OSMO_STRINGIFY(VLR_SUB_AS_AUTH_FAILED),
What I dislike about OSMO_STRINGIFY is that the log output gets unneccessarily long without any gain in information. We already know the type of VLR from the auto generated prefix, so the name "AUTH_FAILED" would be much better than "VLR_SUB_AS_AUTH_FAILED" in the log output. Not a critical issue for merging this, but I do have my reasons for almost never using OSMO_STRINGIFY.
https://gerrit.osmocom.org/#/c/3194/4/src/libvlr/vlr_lu_fsm.c
File src/libvlr/vlr_lu_fsm.c:
Line 984: if (1) { // FIXME
a more verbose description of the FIXME would make sense. I don't really know what is meant here.
--
To view, visit https://gerrit.osmocom.org/3194
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie303c98f8c18e40c87c1b68474b35de332033622
Gerrit-PatchSet: 4
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-HasComments: Yes