Attention is currently required from: neels. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-upf/+/27529 ) Change subject: libosmo-tlv: add TLIV capability ...................................................................... Patch Set 2: (4 comments) File include/osmocom/tlv/tlv.h: https://gerrit.osmocom.org/c/osmo-upf/+/27529/comment/ddb32da7_2b632892 PS2, Line 56: struct osmo_tlv_cfg { why not storing here "instance_present = true/false", then you don't need to specify for each and every TLV you parse and write? I don't think this changes during the same protocol. It really matches here also the "tl_min_size" below since it affects it. Even better, have a "instance_size" which is > 0 if present, and 0 otherwise. You can even then drop the struct osmo_tlv_tag_inst struct and pass "t" and "i" as different params. If protocol doesn't use "i", pass whatever it will be ignored vased on "instance_size == 0". File src/libosmo-tlv/tlv_gen.c: https://gerrit.osmocom.org/c/osmo-upf/+/27529/comment/a31372c0_1c735d82 PS2, Line 36: const struct osmo_tlv_gen_ie osmo_tlv_gen_ie_auto = {0}; This looks not related to this commit. https://gerrit.osmocom.org/c/osmo-upf/+/27529/comment/d3309033_f217c413 PS2, Line 320: printi("{}\n"); not related to this commit? https://gerrit.osmocom.org/c/osmo-upf/+/27529/comment/25e151a1_d24f496a PS2, Line 358: printf("\t{}\n};\n"); not related to this commit? -- To view, visit https://gerrit.osmocom.org/c/osmo-upf/+/27529 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-upf Gerrit-Branch: master Gerrit-Change-Id: I0a076e54dfba6038cc779cb7c8f3967d212226aa Gerrit-Change-Number: 27529 Gerrit-PatchSet: 2 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Comment-Date: Thu, 17 Mar 2022 16:01:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment