isisd: fix crash in display srv6 sid structure in json

FRRouting / frr

The FRRouting Protocol Suite

https://frrouting.org/

Other

3.13k stars 1.21k forks source link

isisd: fix crash in display srv6 sid structure in json #16267

Closed louis-6wind closed 3 weeks ago

louis-6wind commented 3 weeks ago

Fix a crash when doing "show isis database detail json" in isis_srv6_topo1 topotest.
Rename a variable that was incorrectly named, leading to confusion and causing the crash bug
fix a JSON key

Fixes: 2e670cd779 ("isisd: fix display of srv6 subsubtlvs") Fixes: 648a158802 ("isisd: Add SRv6 End.X SID to Sub-TLV format func")

ton31337 commented 3 weeks ago

Can this https://github.com/FRRouting/frr/pull/16267/commits/f912eb036b63d45cbb364683e178b4175fb24fe9 commit be in a separate PR (to backport it to 9.1, 10.0)? Or are you planning to do a backports manually?

louis-6wind commented 3 weeks ago

I will do it manually

louis-6wind commented 3 weeks ago

Can this f912eb0 commit be in a separate PR (to backport it to 9.1, 10.0)? Or are you planning to do a backports manually?

What I said in the commit log was incorrect, there is no crash in 10.0 etc. But the issue in previous versions is that only last key is displayed. The key should in the list element. I will not fix 10.0 because it has the same issue at many places in "show database detail json"

However, I have found a crash fix that was not backported in 10.0. Done in https://github.com/FRRouting/frr/pull/16268

ton31337 commented 3 weeks ago

Can this f912eb0 commit be in a separate PR (to backport it to 9.1, 10.0)? Or are you planning to do a backports manually?

What I said in the commit log was incorrect, there is no crash in 10.0 etc. But the issue in previous versions is that only last key is displayed. The key should in the list element. I will not fix 10.0 because it has the same issue at many places in "show database detail json"

However, I have found a crash fix that was not backported in 10.0. Done in #16268

https://github.com/FRRouting/frr/pull/16267/commits/c06fb90b63ccf2c4d1220f4d8e4b423bc4fc6a40 this needs to be backported for 9.1 too, right?

louis-6wind commented 3 weeks ago

c06fb90 this needs to be backported for 9.1 too, right?

No c06fb90 is for 10.1 branch and master.

For previous versions, 1c397942a46999a850ada62ea015a658373150c6 needs to backported. It is done in https://github.com/FRRouting/frr/pull/16268 for 10.0. The same commit needs to backported in 9.1 and 9.0 as well.