Attention is currently required from: pespin, msuraev.

View Change

9 comments:

• File src/host/layer23/include/osmocom/bb/common/vty.h:

  • Patch Set #8, Line 6: #include <osmocom/vty/command.h>

    only vty.h needs to be included for _LAST_OSMOVTY_NODE, right?

  • Patch Set #8, Line 28:

    (trailing blank line)

• File src/host/layer23/include/osmocom/bb/modem/vty.h:

  • Patch Set #8, 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:

  • Patch Set #8, Line 2: * (C) 2010 by Andreas Eversberg <jolly@eversberg.eu>

    old (C) for new file?

  • Patch Set #8, 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)

  • Patch Set #8, 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:

  • Patch Set #8, 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:

  • Patch Set #8, Line 2: * (C) 2010 by Andreas Eversberg <jolly@eversberg.eu>

    (old (C) for new file)

  • Patch Set #8, Line 58:

    (extra blank line)

To view, visit change 30961. To unsubscribe, or for help writing mail filters, visit settings.