Replace faulty solution to translate YANG union to SNMP

[paul-sirin]

This PR replaces faulty solution to translate YANG union type to SNMP type:

• Revert previous solution to translate YANG union into SNMP type that had memory leak due to cbuf allocation
• Add simple mapping from YANG union to SNMP string

[paul-sirin]

If it is just the cbuf leak it could easily be mitigated with a cbuf_free() after its use on line . 214. What is the logic for removing the sub-type check? Is it un-necessary?

The sub-type check was evaluated as over-engineering and for the most cases it should be ok to fallback to 'string' type

[olofhagsand]

The CI failures have to do with the more ambitious union testing. You say that is over-engineered, I assume then that the regression test should be dropped. Do you have any other way to back that statement, in what way is the union sanity test over-engineered? (the leak is a good catch)

[paul-sirin]

@olofhagsand do I understand correctly that integers are interpreted as strings of bytes and every byte is encoded by setting bits 5 and 6? setting int = 4 results hex string = 34 00 setting int = 2147483647 results hex string = 32 31 34 37 34 38 33 36 34 37 00

[olofhagsand]

I dont think so, where do you see that, can you provide an example? I use the libsnmp lib-functions mostly for those things.

[paul-sirin]

I dont think so, where do you see that, can you provide an example? I use the libsnmp lib-functions mostly for those things.

see failed test https://github.com/clicon/clixon/actions/runs/6707062396/job/18224931237?pr=461 I've changed test data set from 4 to in32 max and wrote an example in previous message UPD: I've got that -- it's just hex of ascii numeric 0x30 to 0x39, shame on me :-D

[paul-sirin]

@olofhagsand I need your suggestion about this change. It works for us well for now because we don't use YANG unions with non-strings, but it will create a technical debt till the time we start to use. So here we might want to fix leak 1st, because it's a major issue (at least for us), but after that we still need another solution to translate YANG unions to SNMP types, because the current one has significant performance issue so we definitely want to replace it. I would appreciate for any help with such solution. We could continue technical discussion on matrix channel.

[olofhagsand]

Yes, regarding the leak: I can easily add that patch. Unless you want to make a separate PR for that?

[paul-sirin]

Yes, regarding the leak: I can easily add that patch. Unless you want to make a separate PR for that?

I will update this one to contain only fix for leak

[shmuelhazan]

@paul-sirin I think we can close this PR, correct?