compile failure in `bgpd/bgp_pbr.c`

[eqvinox]

bgpd/bgp_pbr.c: In function ‘bgp_pbr_icmp_action’:
bgpd/bgp_pbr.c:2029:23: error: storing the address of local variable ‘dstp’ in ‘*bpf.dst_port’ [-Werror=dangling-pointer=]
 2029 |         bpf->dst_port = &dstp;
      |         ~~~~~~~~~~~~~~^~~~~~~
bgpd/bgp_pbr.c:2017:41: note: ‘dstp’ declared here
 2017 |         struct bgp_pbr_range_port srcp, dstp;
      |                                         ^~~~
bgpd/bgp_pbr.c:2017:41: note: ‘bpf’ declared here

$ gcc -v 2>&1 | tail -1  
gcc version 12.2.0 (Debian 12.2.0-1) 

Code dates back to:

commit da3fa38394adad5b892f74fd9009bd7cde76404c
Author: Philippe Guibert <[email protected]>
Date:   Wed Jun 20 16:59:17 2018 +0200

    bgpd: rework icmp enumerate list

Not immediately obvious to me how to fix, otherwise I would've opened a PR instead of an issue.

[mjstapp]

interesting - it doesn't seem wrong to use local/stack variables for local processing. is the error bing reported because the 'bpf' argument returns from "icmp_action" with those pointers set (to stack addresses)? if we cleared those pointers from 'bpf' at function exit, would that resolve the error?

[eqvinox]

if we cleared those pointers from 'bpf' at function exit, would that resolve the error?

Yes, setting src_port/dst_port to NULL at the end of the function makes the warning go away. I'm assuming that's the "correct" fix, but haven't verified that the values aren't being used outside the function(?)

[pguibert6WIND]

if you set to null at the end of the function, after having called bgp_pbr_policyroute_add_to_zebra_unit() or bgp_pbr_policyroute_remove_from_zebra_unit(), this should be ok.

[github-actions[bot]]

This issue is stale because it has been open 180 days with no activity. Comment or remove the autoclose label in order to avoid having this issue closed.

[frrbot[bot]]

This issue will be automatically closed in the specified period unless there is further activity.