Attention is currently required from: pespin. lynxis lazus has posted comments on this change by lynxis lazus. ( https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309?usp=email )
Change subject: Add TCAP based loadsharing/routing ...................................................................... Patch Set 8: (10 comments) Patchset: PS8: @pespin: I like to keep most of the #ifdef / #endif out of the code. The header file already takes care if the feature isn't enabled. Why do you want to have the #ifdef everywhere in the code? The header file transform it into no-ops. File examples/sccp_demo_user.c: https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/b57d649e_3fcadeec?usp=email : PS8, Line 30: void *talloc_asn1_ctx; > why is this added? Because the library needs it. File src/ipa.c: https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/cc96cf5f_86b020ec?usp=email : PS8, Line 315: /* update TCAP Transaction Tracking (Rx) */ > #ifdef WITH_TCAP_LOADSHARING no, not needed. https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/ccabd2eb_642d91c7?usp=email : PS8, Line 316: ss7_asp_tcap_rx_sccp(as, asp, opc, dpc, msg); > This function naming is a bit misleading since it's not even known whether > there's TCAP in the msg.. […] I want to keep the code changes to the other files minimal. What would you suggest instead? https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/140bbf84_334c501d?usp=email : PS8, Line 360: case IPAC_PROTO_OSMO: > #ifdef WITH_TCAP_LOADSHARING I could move it further upwards. https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/349533d8_c8e32076?usp=email : PS8, Line 368: if (hh_ext->proto == IPAC_PROTO_EXT_TCAP_ROUTING) { > (probably a switch case here will turn out cleaner together with the ifdef. > […] yes, I can use switch/case there. File src/m3ua.c: https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/763c4907_f6e3e619?usp=email : PS8, Line 50: #include "ss7_as_loadshare_tcap.h" > #ifdef WITH_TCAP_LOADSHARINGActually, I don't see this header being used in > this file. […] I prefer the style how it is currently. It reduces the usage of #ifdef in the common code. https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/125b9129_31648330?usp=email : PS8, Line 343: if (!xua) > This looks unrelated to this commit? Seems you want to add another commit > describing this. Ack. File tests/tcap/tcap_transaction_tracking_test.c: https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/6c74d55e_52a733af?usp=email : PS6, Line 19: typedef void (* tcap_trxn_track_test_func_t)(void); > space prohibited after that '*' (ctx:BxW) fixed https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/7aa05f39_9a42b3da?usp=email : PS6, Line 19: typedef void (* tcap_trxn_track_test_func_t)(void); > do not add new typedefs Won't fix. I think it's fine here. -- To view, visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: libosmo-sigtran Gerrit-Branch: master Gerrit-Change-Id: Ibcb48aa0e515ad346f59ddd84b24c6e2c026144d Gerrit-Change-Number: 41309 Gerrit-PatchSet: 8 Gerrit-Owner: lynxis lazus <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <[email protected]> Gerrit-Attention: pespin <[email protected]> Gerrit-Comment-Date: Mon, 10 Nov 2025 17:05:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <[email protected]>
