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 <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: laforge <[email protected]> Gerrit-Comment-Date: Sun, 14 Jul 2019 00:17:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
