osmo-mgw[master]: vty: do not change number_endpoints at runtime

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

Neels Hofmeyr gerrit-no-reply at lists.osmocom.org
Fri Nov 10 17:07:17 UTC 2017


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 <pmaier at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-HasComments: Yes



More information about the gerrit-log mailing list