Neels Hofmeyr has posted comments on this change. ( https://gerrit.osmocom.org/11668 )
Change subject: cosmetic: lchan: introduce sub-struct lchan->release.* ...................................................................... Patch Set 1: (3 comments) https://gerrit.osmocom.org/#/c/11668/1/include/osmocom/bsc/gsm_data.h File include/osmocom/bsc/gsm_data.h: https://gerrit.osmocom.org/#/c/11668/1/include/osmocom/bsc/gsm_data.h@495 PS1, Line 495: char *last_error; > Isn't this one related to release too? it gets set for any lchan_fail() invocation, so its scope is broader. Naturally, if an lchan has failed in any way, the usual effect is that it will end up being released, but that's just incidental. The error might be a missing Chan Activ ACK or whatever. https://gerrit.osmocom.org/#/c/11668/1/include/osmocom/bsc/gsm_data.h@518 PS1, Line 518: bool release_requested; > Since it's now inside a "release" name, it makes sense to remove the > "release_" prefix from this var […] I considered the same, but in the end decided that a variable name of "requested" is a bit weird. I agree it should be release.requested and not release.release_requested, and there is no way to access this variable without the "release." prefix. Though, the confusion comes up when you're reading this header: "struct { requested - wtf? - ... oh it says release below". If you/others prefer dropping "release_" then I'll do that. https://gerrit.osmocom.org/#/c/11668/1/include/osmocom/bsc/gsm_data.h@523 PS1, Line 523: /* RSL error code, RSL_ERR_* */ > No enum for RSL_ERR_*? (I know it's not related to this patch). yes, I thought the same, and I agree it sucks. Hysterical raisins. -- To view, visit https://gerrit.osmocom.org/11668 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icfddc6010e5d7c309f1a7ed3526b5b635ffeaf11 Gerrit-Change-Number: 11668 Gerrit-PatchSet: 1 Gerrit-Owner: Neels Hofmeyr <[email protected]> Gerrit-Reviewer: Jenkins Builder (1000002) Gerrit-Reviewer: Neels Hofmeyr <[email protected]> Gerrit-CC: Pau Espin Pedrol <[email protected]> Gerrit-Comment-Date: Wed, 14 Nov 2018 16:11:22 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No
