Change in ...osmo-bsc[master]: add vty 'no neighbors' to remove all HO targets

laforge gerrit-no-reply at lists.osmocom.org
Sun Jul 14 00:17:18 UTC 2019


laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/14768 )

Change subject: add vty 'no neighbors' to remove all HO targets
......................................................................


Patch Set 1:

(4 comments)

https://gerrit.osmocom.org/#/c/14768/1/src/osmo-bsc/neighbor_ident_vty.c 
File src/osmo-bsc/neighbor_ident_vty.c:

https://gerrit.osmocom.org/#/c/14768/1/src/osmo-bsc/neighbor_ident_vty.c@392 
PS1, Line 392: static int del_all(struct vty *vty)
let's make this a bit more descriptive like neighbor_del_all, neigh_del_all, bts_remove_all_neighbors, or the like.


https://gerrit.osmocom.org/#/c/14768/1/src/osmo-bsc/neighbor_ident_vty.c@397 
PS1, Line 397: 
             : 	if (vty->node != BTS_NODE) {
             : 		vty_out(vty, "%% Error: cannot remove BTS neighbor, not on BTS node%s",
             : 			VTY_NEWLINE);
             : 		return CMD_WARNING;
             : 	}
I would actually OSMO_ASSERT here, as this would mean that the VTY command was registered to the wrong node, a clear "must not happen" situation.


https://gerrit.osmocom.org/#/c/14768/1/src/osmo-bsc/neighbor_ident_vty.c@403 
PS1, Line 403: if (!bts) {
             : 		vty_out(vty, "%% Error: cannot remove BTS neighbor, no BTS on this node%s",
             : 			VTY_NEWLINE);
             : 		return CMD_WARNING;
             : 	}
it's arguable whether the removal of 0 neighbors is successful or an error (warning esentially is the error, as they only altrenative is CMD_FATAL which terminates the process).


https://gerrit.osmocom.org/#/c/14768/1/src/osmo-bsc/neighbor_ident_vty.c@410 
PS1, Line 410: while (1) {
             : 		struct gsm_bts_ref *neigh = llist_first_entry_or_null(&bts->local_neighbors, struct gsm_bts_ref, entry);
why are we not using llist_for_each_entry() here? It seems more natural to me.  Actually, as we are deleting members, llist_for_each_entry_safe() would be the more "natural" choice here.  Using while(1) loops always makes me twitchy, for the off chance they could never terminate...



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

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I8623ab581639e9f8af6a9ff1eca990518d1b1211
Gerrit-Change-Number: 14768
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: laforge <laforge at gnumonks.org>
Gerrit-Comment-Date: Sun, 14 Jul 2019 00:17:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190714/ac628bde/attachment.html>


More information about the gerrit-log mailing list