Harald Welte has posted comments on this change. ( 
https://gerrit.osmocom.org/9671 )

Change subject: large refactoring: use FSMs for lchans; add inter-BSC HO
......................................................................


Patch Set 6:

(5 comments)

https://gerrit.osmocom.org/#/c/9671/6//COMMIT_MSG
Commit Message:

https://gerrit.osmocom.org/#/c/9671/6//COMMIT_MSG@35
PS6, Line 35: Thus I can log with or without
            :   typing "\n" and always get an \n termination anyway
I'm not sure if that's such a great feature.  I agree we can talk about a 
policy-change to discontinue the LOGPC continuation.  But I would still suggest 
that programmers explicitly terminate all log lines with \n.  IF we want, we 
could have a "make check" step that validates the source code for that.  
Wireshark is doing things like that at compile/check time, rather than adding 
[potential] runtime.

If we now introduce this magic, I fear that people will accidentially introduce 
log statements without newline.


https://gerrit.osmocom.org/#/c/9671/6/include/osmocom/bsc/gsm_data.h
File include/osmocom/bsc/gsm_data.h:

https://gerrit.osmocom.org/#/c/9671/6/include/osmocom/bsc/gsm_data.h@37
PS6, Line 37: #define GSM_T3122_DEFAULT 10
I guess we don't ned to define this again (it is defined two lines below)


https://gerrit.osmocom.org/#/c/9671/6/include/osmocom/bsc/gsm_data.h@155
PS6, Line 155: mo
I'm still wondering about terminology. It's "handing-out" or "handing-in" or 
something the like, but MO/MT don't really have 
pre-conceived/general/unabiguous meaning in the context of hand-over, at least 
not that I'm aware from reading specs and books.


https://gerrit.osmocom.org/#/c/9671/6/include/osmocom/bsc/gsm_data.h@262
PS6, Line 262:
Not critical, but I don't really like that the old code had extensive comments 
and the new code doesn't have any.


https://gerrit.osmocom.org/#/c/9671/6/include/osmocom/bsc/gsm_data.h@518
PS6, Line 518:  uint8_t error_cause;
might be good to mention that this refers to RSL cause values?



--
To view, visit https://gerrit.osmocom.org/9671
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: I82e3f918295daa83274a4cf803f046979f284366
Gerrit-Change-Number: 9671
Gerrit-PatchSet: 6
Gerrit-Owner: Neels Hofmeyr <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr <[email protected]>
Gerrit-CC: Harald Welte <[email protected]>
Gerrit-Comment-Date: Mon, 25 Jun 2018 13:38:47 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No

Reply via email to