Attention is currently required from: pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-upf/+/27529 )
Change subject: libosmo-tlv: add TLIV capability ...................................................................... Patch Set 3: (2 comments) File include/osmocom/tlv/tlv.h: https://gerrit.osmocom.org/c/osmo-upf/+/27529/comment/18a653ab_85e7877a 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 ev […] struct osmo_tlv_tag_inst is there because "t" and "i" are different params. Both tag and inst are needed to find the destination of a decoded value; i first started out to add the 'instance' arg all over the place, and code dup/bloat spawned in various places. That got annoying, hence this struct to pass and cmp both at the same time. It does not matter whether all tags of a protocol are TLIV or only some, what size the instance value is or which tags have one does not need to be part of the cfg. That is up to the functions loading and storing the TL / TLI headers. The tlv code merely needs to provide storage and match for the instance value. You may be right that we don't need the instance_present flag. The only thing this flag allows is to have the same tag once with and once without instance within the same protocol, that also uses 0 as an instance index ... rather obscure. I added it out of a feeling of not wanting to taint the instance index value. guess i can drop it File src/libosmo-tlv/tlv_gen.c: https://gerrit.osmocom.org/c/osmo-upf/+/27529/comment/283c50e3_2df5b54d PS2, Line 320: printi("{}\n"); > not related to this commit? loosely related, because now that the struct starts with a sub-struct (ti), "{0}" causes a failure ("missing braces around initializer"). Which btw does after all reaffirm me in my original preference for writing "{}". Here we'd have to write "{{0}}", so it turns out "{0}" doesn't always work, but "{}" always works. Anyway, can go to prev commit, yes -- 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: 3 Gerrit-Owner: neels <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: pespin <[email protected]> Gerrit-Attention: pespin <[email protected]> Gerrit-Comment-Date: Thu, 17 Mar 2022 18:10:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <[email protected]> Gerrit-MessageType: comment
