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

laforge gerrit-no-reply at
Sun Jul 14 00:17:18 UTC 2019

laforge has posted comments on this change. ( )

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

Patch Set 1:

File src/osmo-bsc/neighbor_ident_vty.c: 
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. 
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. 
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). 
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
To unsubscribe, or for help writing mail filters, visit

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I8623ab581639e9f8af6a9ff1eca990518d1b1211
Gerrit-Change-Number: 14768
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofmeyr at>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: laforge <laforge at>
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: <>

More information about the gerrit-log mailing list