Attention is currently required from: fixeria. pespin has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-bts/+/41650?usp=email )
Change subject: bts-trx: Convert TRXC and TRXD sockets to iofd ...................................................................... Patch Set 2: (8 comments) File src/osmo-bts-trx/trx_if.c: https://gerrit.osmocom.org/c/osmo-bts/+/41650/comment/29f9ef2b_5d3afbd1?usp=email : PS2, Line 1298: trx_udp_open > I still think having a helper function is better to avoid code duplication > and copy-paste errors. I prefer it the way it is, to have the setup of both together instead of having to jump from the helepr function to here, which was a pain during working on this patch.Also In the end most of the lines contain different parameters for each iofd. https://gerrit.osmocom.org/c/osmo-bts/+/41650/comment/77274369_458a446b?usp=email : PS2, Line 147: msg->data_len > `msgb_length(msg)` here and below No, this is msg->data_len, no msg->len. https://gerrit.osmocom.org/c/osmo-bts/+/41650/comment/89fe2fd5_37ef7457?usp=email : PS2, Line 1006: LOGPPHI(l1h->phy_inst, DTRX, LOGL_ERROR, : > ```suggestion […] Done https://gerrit.osmocom.org/c/osmo-bts/+/41650/comment/3880debe_400923d0?usp=email : PS2, Line 1110: trx_if_send_burst > This proposal looks good on paper, but not easy to implement. Here's a > simpler solution: […] ^ Yes this commit is what I was planning to implement too. https://gerrit.osmocom.org/c/osmo-bts/+/41650/comment/df58690c_d950a0a6?usp=email : PS2, Line 1128: buf = msgb_data(trx_data_last_msg); > setting `buf` here is probably not needed? It is strictly not needed indeed, but given the complexity with static variables I prefer being on the safe case so that it becomes clear that buf is always set, even on the "goto sendall" path below, to avoid also potential false positives from static analysis. https://gerrit.osmocom.org/c/osmo-bts/+/41650/comment/aeea7ebc_c12e30b0?usp=email : PS2, Line 1164: > ```suggestion […] Done https://gerrit.osmocom.org/c/osmo-bts/+/41650/comment/5866c9f8_b2bb2011?usp=email : PS2, Line 1195: LOGPPHI(l1h->phy_inst, DTRX, LOGL_ERROR, : "send() failed on TRXD with rc=%d (%s)\n", : > We're no longer calling `send()` here, right? […] Done https://gerrit.osmocom.org/c/osmo-bts/+/41650/comment/92ad945e_07fc2612?usp=email : PS2, Line 1319: i > Copy-paste! […] Done -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/41650?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I239f91efad43eabd280caf9f852c3aefbc729eaf Gerrit-Change-Number: 41650 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <[email protected]> Gerrit-Attention: fixeria <[email protected]> Gerrit-Comment-Date: Fri, 12 Dec 2025 11:36:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria <[email protected]>
