Change in osmo-pcu[master]: pcu_l1_if_phy: flexible phy access

Attention is currently required from: fixeria, dexter. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/31341 ) Change subject: pcu_l1_if_phy: flexible phy access ...................................................................... Patch Set 5: (5 comments) File src/osmobts_sock.c: https://gerrit.osmocom.org/c/osmo-pcu/+/31341/comment/ecc9bf19_708699b9 PS5, Line 115: bts->trx[trx].fl1h = NULL; After another looks, this looks even worse, because you are not skipping setting fl1h to NULL if phy_ops is not defined. Are you shure that's fine? I'd really do a 2 level if() first checking for fl1h and later calling the function ig phy_ops.l1if_close_pdch exists. File src/pcu_l1_if.cpp: https://gerrit.osmocom.org/c/osmo-pcu/+/31341/comment/44c26336_8f8c36b3 PS5, Line 218: the_pcu->phy_ops->l1if_pdch_req(bts->trx[trx].fl1h, ts, 0, fn, arfcn, block_nr, guard this with "if (the_pcu->phy_ops->l1if_pdch_req)". Similar for all other function calls. File src/pcu_l1_if_phy.c: https://gerrit.osmocom.org/c/osmo-pcu/+/31341/comment/3baf3944_2dc76b1d PS5, Line 15: struct llist_head list; You can really put this inside ops too, and get rid of l1if_phy_entry. https://gerrit.osmocom.org/c/osmo-pcu/+/31341/comment/9730a7aa_e15775a2 PS5, Line 18: const char *name; this can really be in li1f_phy_ops and assign the pointer statically, there's no need to have it separate. IT makes more sense to have it inside the ops since it goes together, it identifies a given set of ops. https://gerrit.osmocom.org/c/osmo-pcu/+/31341/comment/0c9ea33c_53bae5dc PS5, Line 27: bool singleton; this too can be set in phy_ops. I don't see the need of passing several params to register(), one is enough. -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/31341 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I8692d1bd5d137a17cf596ee2914722f419c9978d Gerrit-Change-Number: 31341 Gerrit-PatchSet: 5 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Thu, 16 Feb 2023 14:33:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment