Cleanup of memory allocation and usage of events

[donaldsharp]

Cleanup of both memory allocation and usage of events across of FRR to make the whole thing be consistent.

a) ldpd: event cleanup of unnecessary pointer setting for events, fix bug using malloc instead of calloc
b) lib: Fix null deref in frr_signal_timer
c) ospfd: Cleanup of ospf_apiserver usage of XMALLOC to XCALLOC
d) bgpd, use event_cancel instead of NULL setting in rfapi code
e) pimd, use event_cancel instead of NULL setting.
f) zebra, use event cancel instead of NULL setting.
g) Drop redundant Null setting inside of event handlers
h) Drop redundant if guards before event_cancel calling
i) Use event_is_scheduled() for truthiness of event being active or not
j) Don't need to check for NULL when using alloc functions
k) Create a test that shows bgp bmp mirroring is broken
l) Fix broken bmp mirroring.

[Jafaral]

@greptile review

[greptile-apps]

Greptile Summary

This PR is a broad cleanup across ~120 files, standardising event-pointer truth tests to event_is_scheduled(), removing redundant manual NULL assignments in event callbacks (which the framework now handles), and upgrading bare NULL checks before event_cancel() to unconditional calls. Three commits also fix real bugs: a NULL-deref in lib/sigevent.c, a multi-queue corruption crash in bgpd/bgp_bmp.c (with a new topotest), and a timer leak in bgpd/rfapi/rfapi_import.c.

• Mechanical sweep: ~100 files replace if (t) / t = NULL patterns with event_is_scheduled(t) and drop now-redundant manual NULL clears at callback entry, consistent with the FRR event-system contract.
• bgpd/bgp_bmp.c: Fixes a crash where a single qitem was enqueued into every matching VRF's mirror queue; the fix allocates one qitem per VRF with a correctly scoped refcount.
• lib/sigevent.c: Fixes a definite NULL-deref — the old code set sigm->t = NULL then immediately dereferenced it; the fix uses the t parameter passed to the callback instead.
• ospfd/ospf_apiserver.c: Switches XMALLOC → XCALLOC for two allocations, removing now-redundant explicit zero-inits; new->filter->origin = ANY_ORIGIN (value 2) is still set explicitly since it is non-zero.

Confidence Score: 4/5

The PR is safe to merge. All three substantive bug fixes (sigevent NULL-deref, BMP mirror multi-queue corruption, rfapi timer leak) are correct, and the new topotest covers the BMP crash scenario.

The changes are correct and the real fixes are well-motivated, but the sweep touches ~120 files across nearly every FRR daemon, which means any subtle semantic difference between t != NULL and event_is_scheduled(t) could introduce a latent issue in a path not covered by CI. The new topotest validates the most impactful fix.

The bgpd/bgp_bmp.c refcount logic in bmp_mirror_packet is the most structurally significant change and warrants a careful second read; the rest of the files are mechanical and low-risk.

Important Files Changed

Filename	Overview
lib/sigevent.c	Fixes a NULL-deref crash: old code set sigm->t = NULL then used sigm->t->master; fix correctly uses the callback's own event parameter t->master.
bgpd/bgp_bmp.c	Fixes crash where a single qitem was enqueued into multiple VRF mirror queues; now allocates a fresh qitem per-VRF with correctly scoped refcount. Also removes redundant t_read = NULL and adds event_is_scheduled checks.
bgpd/rfapi/rfapi_import.c	Replaces bare timer = NULL assignments before event_add_timer with event_cancel(), properly cancelling any running timer before re-arming instead of leaking it.
ospfd/ospf_apiserver.c	Switches to XCALLOC (zero-init) removing redundant field NULLing; retains explicit origin = ANY_ORIGIN (value 2) since it is non-zero. Removes unreachable NULL check after stream_new.
lib/nexthop.c	Removes unreachable NULL-check branch after XREALLOC; FRR's allocator aborts on OOM so the fallback path (clamping new_stack->num_labels) was dead code.
pimd/pim_pim.c	Replaces bare t = NULL with event_cancel() in pim_sock_add, ensuring any existing read event is properly cancelled when the socket is recreated.
tests/lib/test_memory.c	Removes dead NULL check after XREALLOC; FRR's XREALLOC never returns NULL (aborts on OOM), so the conditional copy was unreachable.
bgpd/bgp_fsm.c	Mechanical: replaces all bare event-pointer NULL tests with event_is_scheduled() calls across GR, LLGR, routeadv, maxmed, and delayopen timer checks.
tests/topotests/bgp_bmp/test_bgp_bmp_mirror.py	New topotest that reproduces the multi-target BMP mirror crash with two bmp targets sharing a single BGP instance; validates queue integrity and session health.
lib/zclient.c	Drops redundant t_write = NULL and t_read = NULL at callback entry; removes spurious extra NULL in BUFFER_PENDING case; uses event_is_scheduled for t_connect guard in zclient_start.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["bmp_mirror_packet(peer, ...)"] --> B["For each bgp_vrf in bm->bgp"]
    B --> C{"bmpbgp = bmp_bgp_find(bgp_vrf)\nfound?"}
    C -- No --> B
    C -- Yes --> D["qitem = NULL  (per-VRF local)"]
    D --> E["For each bmp_targets bt with mirror=true"]
    E --> F{"bgp_vrf matches\npeer->bgp?"}
    F -- No --> E
    F -- Yes --> G["For each bmp session"]
    G --> H{"qitem == NULL?"}
    H -- Yes --> I["XCALLOC new qitem\nset peerid/tv/len/data"]
    I --> J["refcount++\nmirrorpos = qitem\npullwr_bump"]
    H -- No --> J
    J --> G
    G -- done --> K{"qitem != NULL?"}
    K -- No --> B
    K -- Yes --> L["Add to bmpbgp->mirrorq\nbmp_mirror_cull\nupdate qsizemax"]
    L --> B
    B -- done --> M["return 0"]

Loading

_{Reviews (1): Last reviewed commit: "bgpd: Fix crash in bgp_bmp.c with a mirr..." | Re-trigger Greptile}

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[riw777]

looks good