Attention is currently required from: jolly, laforge, dexter.

pespin has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-mgw/+/33548 )

Change subject: ASCI: Support conference briding with 1..n connections
......................................................................


Patch Set 4:

(4 comments)

File src/libosmo-mgcp/mgcp_conn.c:

https://gerrit.osmocom.org/c/osmo-mgw/+/33548/comment/f7d32a89_b9f6f836
PS4, Line 176:  /* Do not allow more then the maximum number of connections */
you could take the cance to fix the "then" typo :D


https://gerrit.osmocom.org/c/osmo-mgw/+/33548/comment/6d312e44_abc1d612
PS4, Line 177:  if (endp->type->max_conns && llist_count(&endp->conns) >= 
endp->type->max_conns)
max_conns > 0 may be clearer here.


File src/libosmo-mgcp/mgcp_network.c:

https://gerrit.osmocom.org/c/osmo-mgw/+/33548/comment/6e1dba6b_41c28cd2
PS4, Line 1329:         if (conn->mode != MGCP_CONN_RECV_ONLY &&
sounds like you want to use a switch() here? It's difficult to gasp the 
different cases otherwise.


https://gerrit.osmocom.org/c/osmo-mgw/+/33548/comment/d6baae15_aa2a01ab
PS4, Line 1342:                 if (conn_dst->mode == MGCP_CONN_SEND_ONLY ||
Also lots of mode checking here. I bet this all can be improved a lot by 
re-thinking it and using switch statements, etc.



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

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: Ic99a55ab5a3a6170e940403fadd52697e99f2f3a
Gerrit-Change-Number: 33548
Gerrit-PatchSet: 4
Gerrit-Owner: jolly <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <[email protected]>
Gerrit-CC: laforge <[email protected]>
Gerrit-CC: pespin <[email protected]>
Gerrit-Attention: jolly <[email protected]>
Gerrit-Attention: laforge <[email protected]>
Gerrit-Attention: dexter <[email protected]>
Gerrit-Comment-Date: Mon, 10 Jul 2023 08:54:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

Reply via email to