[M] Change in libosmocore[master]: tdef_vty: Introduce helper functions/adapt existing helpers

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. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35328?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ife88841fbc325f23c5b270b8e8c71678b8023639 Gerrit-Change-Number: 35328 Gerrit-PatchSet: 6 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: arehbein <arehbein(a)sysmocom.de> Gerrit-Comment-Date: Thu, 14 Dec 2023 12:21:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment