Attention is currently required from: arehbein.

pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35328?usp=email )

Change subject: tdef_vty: Introduce helper functions/adapt existing helpers ......................................................................

Patch Set 6: Code-Review-1

(8 comments)

File src/vty/tdef_vty.c:

https://gerrit.osmocom.org/c/libosmocore/+/35328/comment/4b404dac_e37fc76e PS6, Line 314: static char *add_group_args(void *talloc_ctx, char *dest, struct osmo_tdef_group *src) src can be const.

https://gerrit.osmocom.org/c/libosmocore/+/35328/comment/1382f2e3_edecf14f PS6, Line 327: static char *add_group_docs(void *talloc_ctx, char *dest, struct osmo_tdef_group *src) src can be const.

https://gerrit.osmocom.org/c/libosmocore/+/35328/comment/7b40574c_1f726ded PS6, Line 340: dest = add_group_args(tall_vty_cmd_ctx, dest, src); src can be const.

https://gerrit.osmocom.org/c/libosmocore/+/35328/comment/0f92c240_9f6e1229 PS6, Line 349: dest = add_group_docs(tall_vty_cmd_ctx, dest, src); src can be const.

https://gerrit.osmocom.org/c/libosmocore/+/35328/comment/97fb4f0a_32d92fda PS6, Line 358: static void set_show_cmd_strs(struct osmo_tdef_group *template, const char *cmd_group, I don't really like the "template" naming, it's not something we have been using so far and imho difficult to gasp. gerrit also seems to agree with it being not a good name as it's marking it as if it was a keyword 😊

template can be const.

https://gerrit.osmocom.org/c/libosmocore/+/35328/comment/a06b9626_e7b4585f PS6, Line 361: size_t cmd_group_len = cmd_group ? strlen(cmd_group) : 0; this is all really hard to read, you are putting to much stuff on any line. Add some spacing and rework it.

Have a look at osmo_strbuf. Too many strlen() everywhere (hardcoded strings can use sizeof()).

I'm pretty sure cmd_group_space won't be needed.

https://gerrit.osmocom.org/c/libosmocore/+/35328/comment/4555d815_f9d6141c PS6, Line 384: static void set_cfg_cmd_strs(struct osmo_tdef_group *template, const char *cmd_group, see comments from set_show_cmd_strs().

https://gerrit.osmocom.org/c/libosmocore/+/35328/comment/ded2af03_4d235e48 PS6, Line 445: strcat(memcpy(cmd_group_space, cmd_group, strlen(cmd_group) + 1), " "); I think it's first time I see somebody using the return value of memcpy. Anyway, all this is too complex, simply use snprintf() or osmo_strbuf.