Patch Set 1: Code-Review-1 (1 comment)
add 'Related: OS#1234' to commit log https://gerrit.osmocom.org/#/c/4779/1/src/libosmo-mgcp/mgcp_vty.c File src/libosmo-mgcp/mgcp_vty.c: Line 511: { hmm, these are fairly complex semantics now. Is it guaranteed that allocation has already happened after the first VTY command was issued? What if the user by accident has two number endpoints commands in the config? The later one will not be applied at runtime, yet be written out during 'write config'. What if there is no 'number endpoints' VTY command in the config at all? Will new_number_endpoints still be -1? It would be far clearer and less error prone to separately remember how much was actually allocated. So instead of adding new_number_endpoints, I'd add a vty_number_endpoints, let the vty always and only write to that, and precisely upon allocating the struct, copy that value over to number_endpoints, to be used by the for() loops. Telling the user that the command does not take immediate effect is nice, maybe you can still do that by checking whether number_endpoints > 0 (i.e. whether something was allocated yet). (We do have numerous such cases where we don't warn the user, so it's no requirement) -- To view, visit https://gerrit.osmocom.org/4779 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3994af016fb96427263edbba05f560743f85fdd4 Gerrit-PatchSet: 1 Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Owner: dexter <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Neels Hofmeyr <[email protected]> Gerrit-HasComments: Yes
