Change in osmo-bsc[master]: cosmetic: lchan: introduce sub-struct lchan->release.*

[Neels Hofmeyr]

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 <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-CC: Pau Espin Pedrol <pespin at sysmocom.de>
Gerrit-Comment-Date: Wed, 14 Nov 2018 16:11:22 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20181114/45c6fbe7/attachment.htm>