Patch Set 5: (8 comments)
I'm not familiar with these parts of the spec. So my comments are only cosmetic in nature. https://gerrit.osmocom.org/#/c/5753/5/src/common/msg_utils.c File src/common/msg_utils.c: Line 388: else Add curly braces to this else block? It spans more than one line due to indentation. I wonder though if we should just remove the 'else' line and then indent the line below it one column to the left. Line 489: if (!!lchan->tch.dtx.len) Is !! intentional? Might be clearer to write this as something like if (len) or if (len == 0) or if (len != 0)? Line 529: /* Debug print */ Comment adds no information and could be removed. Line 565: /* Debug print */ Comment adds no information and could be removed. https://gerrit.osmocom.org/#/c/5753/5/src/osmo-bts-litecell15/l1_if.c File src/osmo-bts-litecell15/l1_if.c: Line 365: /* Debug print */ This comment could be deleted, it doesn't add additional information. Line 504: if (lchan->rsl_cmode == RSL_CMOD_SPD_SIGN) This if-block should use curly braces because it spans more than one line due to the comment Line 508: if (lchan->ts->trx->bts->dtxd) likewise Line 511: else This else block should use curly braces, too. -- To view, visit https://gerrit.osmocom.org/5753 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I40e9bf9438c0b400e4d29eb39ffae37207e34db6 Gerrit-PatchSet: 5 Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Owner: Minh-Quang Nguyen <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Max <[email protected]> Gerrit-Reviewer: Minh-Quang Nguyen <[email protected]> Gerrit-Reviewer: Pau Espin Pedrol <[email protected]> Gerrit-Reviewer: Stefan Sperling <[email protected]> Gerrit-Reviewer: Vadim Yanitskiy <[email protected]> Gerrit-HasComments: Yes
