Attention is currently required from: arehbein, neels, fixeria.
pespin has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-bsc/+/30602 )

Change subject: vty: Add check against sensible default value for Ny1
......................................................................


Patch Set 18: Code-Review-1

(6 comments)

File src/osmo-bsc/bts.c:

https://gerrit.osmocom.org/c/osmo-bsc/+/30602/comment/e26ee03a_3167abcc
PS18, Line 535:         if (!gsm_bts_check_ny1(bts, LOGL_ERROR)) return -EINVAL;
put return always in a separate line.


https://gerrit.osmocom.org/c/osmo-bsc/+/30602/comment/abc2e43e_a1f52a6c
PS18, Line 1726:        const struct gsm_lchan *gl = gsm_bts_get_cbch(bts);
I'm still not understanding why are you taking a CBCH channel here, seems 
totally not related.
Furthermore, it should be called "lchan".


https://gerrit.osmocom.org/c/osmo-bsc/+/30602/comment/c2f726e4_bc292299
PS18, Line 1729:        if (gl) {
no need for {} here. Linter should already have warned about it.


https://gerrit.osmocom.org/c/osmo-bsc/+/30602/comment/a447cd3e_4caa72a9
PS18, Line 1730:                T3124 = (gl->type == GSM_LCHAN_SDCCH) ? 
GSM_T3124_SDCCH : GSM_T3124_OTHER_CH;
this is always going to be false since gl is CBCH.


https://gerrit.osmocom.org/c/osmo-bsc/+/30602/comment/5db249de_80f64f13
PS18, Line 1732:                /* Take the higher lower bound to be safe */
you can then move the comment to the "else" line (1731)


File src/osmo-bsc/bts_ipaccess_nanobts_omlattr.c:

https://gerrit.osmocom.org/c/osmo-bsc/+/30602/comment/0bafb1aa_88aeb5f2
PS18, Line 95:  gsm_bts_check_ny1(bts, LOGL_NOTICE);
I really see no reason for having different log levels.



--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/30602
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: If3f96a6bd4f9ae32b6421de43c1c5a5d64482089
Gerrit-Change-Number: 30602
Gerrit-PatchSet: 18
Gerrit-Owner: arehbein <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <[email protected]>
Gerrit-Reviewer: pespin <[email protected]>
Gerrit-CC: neels <[email protected]>
Gerrit-Attention: arehbein <[email protected]>
Gerrit-Attention: neels <[email protected]>
Gerrit-Attention: fixeria <[email protected]>
Gerrit-Comment-Date: Tue, 03 Jan 2023 10:17:14 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Reply via email to