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 gerrit-no-reply at lists.osmocom.orgneels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/18907 ) Change subject: fix crashes due to OSMO_ASSERT(conn->lchan) ...................................................................... Patch Set 5: (5 comments) thanks for the review! I'm afraid I have to negate every change request made, though... Let's get rid of this osmo-bsc crash by OSMO_ASSERT(conn->bts) now? https://gerrit.osmocom.org/c/osmo-bsc/+/18907/5/include/osmocom/bsc/gsm_data.h File include/osmocom/bsc/gsm_data.h: https://gerrit.osmocom.org/c/osmo-bsc/+/18907/5/include/osmocom/bsc/gsm_data.h@1376 PS5, Line 1376: if (!conn || !conn->lchan) > I'd rather check only for !conn->lchan, I would assume conn is always there and if it's not there is […] no harm done by it, so I'm just keeping this https://gerrit.osmocom.org/c/osmo-bsc/+/18907/5/src/osmo-bsc/assignment_fsm.c File src/osmo-bsc/assignment_fsm.c: https://gerrit.osmocom.org/c/osmo-bsc/+/18907/5/src/osmo-bsc/assignment_fsm.c@83 PS5, Line 83: if (bts) \ > what about logging if !bts? this macro is supposed to silently count rate counters in various situations, if there is no bts then just don't count a bts and done https://gerrit.osmocom.org/c/osmo-bsc/+/18907/5/src/osmo-bsc/gsm_data.c File src/osmo-bsc/gsm_data.c: https://gerrit.osmocom.org/c/osmo-bsc/+/18907/5/src/osmo-bsc/gsm_data.c@1724 PS5, Line 1724: if (bts && conn->lchan) > But we still need the check since we dereference bts->ms_max_power in the next line, so what do we g […] what he said https://gerrit.osmocom.org/c/osmo-bsc/+/18907/5/src/osmo-bsc/osmo_bsc_bssap.c File src/osmo-bsc/osmo_bsc_bssap.c: https://gerrit.osmocom.org/c/osmo-bsc/+/18907/5/src/osmo-bsc/osmo_bsc_bssap.c@477 PS5, Line 477: static int bssmap_handle_cipher_mode(struct gsm_subscriber_connection *conn, > I think it belongs in this patch, but I don't think we should use global variables (like bsc_gsmnet) […] In this function, the only reason to use conn_get_bts() was to get at the global singleton bsc_gsmnet. It is always there, and it makes no sense to not be able to access it just because there currently is no lchan assigned. The fact that this function should always have an lchan aside: the point of this patch is to safeguard against odd failure modes where an lchan disappeared unexpectedly, and to make sure there is no DoS. I was all against a global gsmnet myself for a while, but in practice, there is no use in passing it around as function args. There is only one bsc_gsmnet, there will always be just one, and there is absolutely nothing gained now nor ever in any future by making it complicated to obtain a reference to it. https://gerrit.osmocom.org/c/osmo-bsc/+/18907/5/src/osmo-bsc/osmo_bsc_msc.c File src/osmo-bsc/osmo_bsc_msc.c: https://gerrit.osmocom.org/c/osmo-bsc/+/18907/5/src/osmo-bsc/osmo_bsc_msc.c@239 PS5, Line 239: return NULL; > OSMO_ASSERT(bts) here looks better. no, because getting rid of an assert like that is exactly the aim of this patch. All cgi_for_msc() callers handle a NULL return val properly. -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/18907 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: Id681dfb0ad654bdb4b71805d1ad4f39a8bf6bbd1 Gerrit-Change-Number: 18907 Gerrit-PatchSet: 5 Gerrit-Owner: fixeria <vyanitskiy at sysmocom.de> Gerrit-Assignee: neels <nhofmeyr at sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy at sysmocom.de> Gerrit-CC: daniel <dwillmann at sysmocom.de> Gerrit-CC: neels <nhofmeyr at sysmocom.de> Gerrit-CC: pespin <pespin at sysmocom.de> Gerrit-Comment-Date: Tue, 23 Jun 2020 12:29:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin at sysmocom.de> Comment-In-Reply-To: daniel <dwillmann at sysmocom.de> Gerrit-MessageType: comment -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20200623/486a9c9c/attachment.htm>