Attention is currently required from: pespin, msuraev. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/30961 )
Change subject: layer23: Initial VTY framework to share VTY code between apps ...................................................................... Patch Set 8: (9 comments) File src/host/layer23/include/osmocom/bb/common/vty.h: https://gerrit.osmocom.org/c/osmocom-bb/+/30961/comment/508f8382_a8418d4c PS8, Line 6: #include <osmocom/vty/command.h> only vty.h needs to be included for _LAST_OSMOVTY_NODE, right? https://gerrit.osmocom.org/c/osmocom-bb/+/30961/comment/9b70c6f8_e3809264 PS8, Line 28: (trailing blank line) File src/host/layer23/include/osmocom/bb/modem/vty.h: https://gerrit.osmocom.org/c/osmocom-bb/+/30961/comment/fcd4caa1_dc29abf7 PS8, Line 7: #include <osmocom/bb/common/vty.h> none of the includes are necessary for this .h, need only struct vty; here File src/host/layer23/src/common/vty.c: https://gerrit.osmocom.org/c/osmocom-bb/+/30961/comment/39c6fef8_cadf2d21 PS8, Line 2: * (C) 2010 by Andreas Eversberg <[email protected]> old (C) for new file? https://gerrit.osmocom.org/c/osmocom-bb/+/30961/comment/05231c42_dfdfa71e PS8, Line 172: int l23_vty_go_parent(struct vty *vty) you can completely drop vty_go_parent since libosmocore patch commit d31de237582f6fe3315d61bb9a488d4cda92654e Date: Thu Oct 31 16:09:23 2019 +0100 Subject: vty: track parent nodes also for telnet sessions the only need for a go_parent_cb is if you want to add some very specific action to happen when leaving a node (like restart a socket or something) https://gerrit.osmocom.org/c/osmocom-bb/+/30961/comment/8ba9eeae_67796002 PS8, Line 186: int l23_vty_init(int (*config_write_ms_node_cb)(struct vty *)) > This may be returning 0 only now but it's an API so I prefer keeping return > int and checking result […] funny how in this patch, you don't check the rc of l23_vty_init() calls anywhere. Either check the rc or make it void. File src/host/layer23/src/mobile/vty_interface.c: https://gerrit.osmocom.org/c/osmocom-bb/+/30961/comment/96c45b4f_76551150 PS8, Line 3209: osmo_talloc_vty_add_cmds(); why is this dropped? possibly better in a separate patch? File src/host/layer23/src/modem/vty.c: https://gerrit.osmocom.org/c/osmocom-bb/+/30961/comment/3462d512_3649b1e1 PS8, Line 2: * (C) 2010 by Andreas Eversberg <[email protected]> (old (C) for new file) https://gerrit.osmocom.org/c/osmocom-bb/+/30961/comment/327bbc15_a933a125 PS8, Line 58: (extra blank line) -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/30961 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: Iabfb3129199488d790b89884bc1e424f2aca696f Gerrit-Change-Number: 30961 Gerrit-PatchSet: 8 Gerrit-Owner: pespin <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <[email protected]> Gerrit-CC: msuraev <[email protected]> Gerrit-CC: neels <[email protected]> Gerrit-Attention: pespin <[email protected]> Gerrit-Attention: msuraev <[email protected]> Gerrit-Comment-Date: Sun, 15 Jan 2023 23:54:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <[email protected]> Comment-In-Reply-To: msuraev <[email protected]> Gerrit-MessageType: comment
