Change in osmo-mgw[master]: libosmo-mgcp-client: extend the mgcp_client for MGW pooling

osmith gerrit-no-reply at lists.osmocom.org
Thu Aug 5 12:21:41 UTC 2021


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

Change subject: libosmo-mgcp-client: extend the mgcp_client for MGW pooling
......................................................................


Patch Set 8:

(6 comments)

only some cosmetics, rest looks good! thanks for answering everything in detail, makes sense.

https://gerrit.osmocom.org/c/osmo-mgw/+/25120/5/src/libosmo-mgcp-client/mgcp_client_pool.c 
File src/libosmo-mgcp-client/mgcp_client_pool.c:

https://gerrit.osmocom.org/c/osmo-mgw/+/25120/5/src/libosmo-mgcp-client/mgcp_client_pool.c@159 
PS5, Line 159: 	 *  not belong to a pool at all, the function call will have no effect. */
> I think that is done automatically by doxygen. […]
What I meant is, I'd move this comment above "void mgcp_client_pool_put(...". like here: https://git.osmocom.org/libosmocore/tree/src/timer.c?id=7af860fb78d6a13258d5887e7bb71276509229cb#n114


https://gerrit.osmocom.org/c/osmo-mgw/+/25120/8/src/libosmo-mgcp-client/mgcp_client_pool.c 
File src/libosmo-mgcp-client/mgcp_client_pool.c:

https://gerrit.osmocom.org/c/osmo-mgw/+/25120/8/src/libosmo-mgcp-client/mgcp_client_pool.c@133 
PS8, Line 133: 	 *  by the application code. */
(Move above struct mgcp_client *mgcp_client_pool_get, see other comment?)


https://gerrit.osmocom.org/c/osmo-mgw/+/25120/5/src/libosmo-mgcp-client/mgcp_client_vty.c 
File src/libosmo-mgcp-client/mgcp_client_vty.c:

https://gerrit.osmocom.org/c/osmo-mgw/+/25120/5/src/libosmo-mgcp-client/mgcp_client_vty.c@50 
PS5, Line 50:  \
> I have to use this macro in each (common) VTY command to get the pointer to the conf. […]
but why not a function instead of a macro?


https://gerrit.osmocom.org/c/osmo-mgw/+/25120/5/src/libosmo-mgcp-client/mgcp_client_vty.c@329 
PS5, Line 329:  
> The extra space is on purpose. […]
sorry I got confused about the snprintf arguments. global_mgcp_client_indent_pool gets read, not written to. FWIW I think it is more readable in the new version without the global variable.


https://gerrit.osmocom.org/c/osmo-mgw/+/25120/5/src/libosmo-mgcp-client/mgcp_client_vty.c@531 
PS5, Line 531: indent
> I would normally expect the indent string to be a constant, but who knows where an API user is getti […]
(nvm the strdup comment, I thought global_mgcp_client_indent_pool would get written to... but now I see that it was only read.)


https://gerrit.osmocom.org/c/osmo-mgw/+/25120/8/src/libosmo-mgcp-client/mgcp_client_vty.c 
File src/libosmo-mgcp-client/mgcp_client_vty.c:

https://gerrit.osmocom.org/c/osmo-mgw/+/25120/8/src/libosmo-mgcp-client/mgcp_client_vty.c@38 
PS8, Line 38: connands
commands



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

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: Icaaba0e470e916eefddfee750b83f5f65291a6b0
Gerrit-Change-Number: 25120
Gerrit-PatchSet: 8
Gerrit-Owner: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: osmith <osmith at sysmocom.de>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-Comment-Date: Thu, 05 Aug 2021 12:21:41 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: osmith <osmith at sysmocom.de>
Comment-In-Reply-To: dexter <pmaier at sysmocom.de>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210805/8800e644/attachment.htm>


More information about the gerrit-log mailing list