Change in osmo-bts[master]: osmo-bts-trx: implement TRXDv2 protocol support

[laforge]

laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/23833 )

Change subject: osmo-bts-trx: implement TRXDv2 protocol support
......................................................................


Patch Set 7:

(2 comments)

https://gerrit.osmocom.org/c/osmo-bts/+/23833/7/src/osmo-bts-trx/trx_if.c 
File src/osmo-bts-trx/trx_if.c:

https://gerrit.osmocom.org/c/osmo-bts/+/23833/7/src/osmo-bts-trx/trx_if.c@987 
PS7, Line 987: loop:
> I prefer to use 'goto' here to avoid wrapping large code blocks into { }, and see no problem with us […]
* the indent level here is not deep at all
* the length of the function is the same whether you use a proper loop or a goto

So by all means I am with pespin here.  If you think the code is somehow not reaable enough, then it should be split into smaller functions...


https://gerrit.osmocom.org/c/osmo-bts/+/23833/7/src/osmo-bts-trx/trx_if.c@1114 
PS7, Line 1114: 	LOGPPHI(l1h->phy_inst, DTRX, LOGL_DEBUG,
> I am just moving the existing log line, not introducing a new one. […]
I agree it should not be changed if this code just moves it around.

However, I do think there's little point in logging every TRXDF datagram just before a syscall.  There's strace for such debugging, or of course, pcap/wireshark/tcpdump.

For TRXD you will be overwhelmed by the amount of logging anyway in this situation.

So I think it should be removed in a separate patch, but I'm not saying fixeria has to do that just because he works with this code...



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

Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I3532a6693bb335043ec390049138308991083e66
Gerrit-Change-Number: 23833
Gerrit-PatchSet: 7
Gerrit-Owner: fixeria <vyanitskiy at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-Comment-Date: Fri, 30 Apr 2021 16:19:05 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria <vyanitskiy at sysmocom.de>
Comment-In-Reply-To: pespin <pespin at sysmocom.de>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210430/8a2b38a1/attachment.htm>