Attention is currently required from: fixeria.
pespin has posted comments on this change. (
https://gerrit.osmocom.org/c/osmo-pcu/+/32358 )
Change subject: ms: Make ms_{attach,detach}_tbf expectancies more robust
......................................................................
Patch Set 2:
(3 comments)
File src/gprs_ms.c:
https://gerrit.osmocom.org/c/osmo-pcu/+/32358/comment/7a4cc895_fc963d33
PS2, Line 355: OSMO_ASSERT(ms);
> Somehow it feels wrong when I am seeing a function assert()ing all its
> pointer arguments. […]
it's not really the same. These are internal pointers, not stuff received from
the peers over the wire. They are checking internal state, and help understand
readers (and remind myself) of the expected assumptions.
https://gerrit.osmocom.org/c/osmo-pcu/+/32358/comment/24d56194_50a8847a
PS2, Line 409: OSMO_ASSERT(!ms_tbf_is_attached(ms, tbf));
> ... especially when some calling functions also do assert the arguments too.
Again, this is internal state. If it breaks it means something is fundamentally
wrong in the model, or somebody made a change in the wrong direction. It also
helps understand the current model.
https://gerrit.osmocom.org/c/osmo-pcu/+/32358/comment/4436d8ad_4fe7c7e4
PS2, Line 419: OSMO_ASSERT(tbf_ms(tbf) == ms);
> ... and here you're dereferencing tbf before assert()ing it.
I can add an OSMO_ASSERT(tbf) there if you want, but it's clear here that it is
expected to be not NULL.
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/32358
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: Ia18fe2de1fb3bf72f530157e5f30de64f2b11e12
Gerrit-Change-Number: 32358
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: fixeria <[email protected]>
Gerrit-Attention: fixeria <[email protected]>
Gerrit-Comment-Date: Thu, 20 Apr 2023 18:39:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria <[email protected]>
Gerrit-MessageType: comment
