fix(net/igmp): restore General Query handling broken by pointer compare

[dakejahl]

Summary

igmp_input() never handles IGMP General Queries. The test that distinguishes a General Query from a Group-Specific Query compares the address of a struct member against zero, so it is always false and the General Query branch is unreachable. GCC discards the branch entirely.

The group address is declared in include/nuttx/net/igmp.h as

uint16_t grpaddr[2];       /* 32-bit Group address */

so igmp->grpaddr decays to FAR uint16_t *. Comparing it to INADDR_ANY compares a pointer, not the address the packet carries.

This was reported once before, as a compiler warning rather than as a defect. Commit 09bb292 (#7273, "net/igmp: fix build warning on GCC 12.2.0") changed

if (igmp->grpaddr == 0)

to

if (net_ipv4addr_cmp(igmp->grpaddr, INADDR_ANY) != 0)

net_ipv4addr_cmp(a, b) expands to (a == b) and INADDR_ANY expands to ((in_addr_t)0), so the emitted comparison is unchanged. The -Waddress diagnostic disappeared for a different reason: the comparison now originates inside a macro expanded from a header reached via -isystem, and GCC suppresses warnings from system-header macros. Demonstration with the same expression, macro in a header:

$ gcc -c -O2 -Wall -I sysinc lauder.c
sysinc/nxmacros.h:5:48: warning: comparison between pointer and zero character constant [-Wpointer-compare]

$ gcc -c -O2 -Wall -isystem sysinc lauder.c      # what NuttX's build actually does
$                                                # silent

The defect was hidden rather than fixed, and the report was closed. That commit also rewrote the unicast query test from group->grpaddr != 0, which was well-formed (group->grpaddr is an in_addr_t), into the same pointer comparison, making it unconditionally true.

The fix converts the header field once with net_ip4addr_conv32() and compares the resulting in_addr_t. The conversion was already performed in the group-specific branch, so this only hoists it and reuses it at the other two sites. net_ipv4addr_hdrcmp() is not usable here because it applies net_ip4addr_conv32() to both operands, and the second operand is the scalar INADDR_ANY.

Impact

Affects all users with CONFIG_NET_IGMP=y. No API, configuration, or build-system change; no new symbols. Code size grows by the branch that is currently being discarded (+149 bytes of .text on sim, see below).

A General Query — destination 224.0.0.1, group address 0, per RFC 2236 §2.4 — is the periodic query every IGMP querier emits, typically every 125 s. Today it falls through to the group-specific branch, where igmp_grpallocfind(dev, &grpaddr) is called with grpaddr == 0.0.0.0. That allocates a group entry for 0.0.0.0 and schedules a membership report for it, while the groups the device actually joined never get their report timers restarted. The querier then ages out those memberships and multicast delivery to the device stops.

Testing

Host: Linux x86_64, GCC 13.

Board/config: sim:dynconns, which sets CONFIG_NET_IGMP=y. Also cross-checked the affected expressions with arm-none-eabi-gcc 13.2.1.

tools/checkpatch.sh -c -u -m -g master..HEAD (the invocation CI uses):

Used config files:
    1: .codespellrc
✔️ All checks pass.

sim:dynconns builds cleanly before and after; the file produces no diagnostics either way, since the warning is suppressed as described above. The behavioural change is visible in the generated code instead. igmp->maxresp = 10; occurs only inside the General Query branch, so it is a reliable marker for whether that branch survives:

$ make && cd net && size igmp_input.o && objdump -d igmp_input.o | grep -c 'movb.*\$0xa'

	.text	igmp->maxresp = 10 present
master (073570a103)	510	0 — branch eliminated as unreachable
this PR	659	1

Before this change the compiler proves the General Query handler is dead and removes it; after it, the handler is emitted.

The defect was originally found by a -Werror build of PX4's NuttX fork, which predates 09bb292 and so still carries the original igmp->grpaddr == 0 and still fails to build. That fork is functionally affected in exactly the same way.

Signed-off-by: Jacob Dahl dahl.jakejacob@gmail.com

[github-actions]

MemBrowse Memory Report

hifive1-revb

• flash: .text -4 B (-0.0%, 83,248 B / 4,194,304 B, total: 2% used)
  No memory changes detected for:
• arduino-mega2560
• esp32-devkitc
• mirtoo
• qemu-armv8a
• qemu-intel64
• rx65n-rsk2mb
• s698pm-dkit
• stm32-nucleo-f103rb