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 jolly@eversberg.eu 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 jolly@eversberg.eu (old (C) for new file)
https://gerrit.osmocom.org/c/osmocom-bb/+/30961/comment/327bbc15_a933a125 PS8, Line 58: (extra blank line)