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(a)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(a)eversberg.eu>
(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 <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-CC: msuraev <msuraev(a)sysmocom.de>
Gerrit-CC: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: msuraev <msuraev(a)sysmocom.de>
Gerrit-Comment-Date: Sun, 15 Jan 2023 23:54:03 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: msuraev <msuraev(a)sysmocom.de>
Gerrit-MessageType: comment