[Dnsmasq-discuss] Git: Is first dhcp.c address_available() for/if code correct?

[Matthias Andree]

Simon,

looking at dhcp.c as of this commit, and I see it's unchanged from v2.91:

> commit ee09f0655c0a4347a72d2bf9b7231ff158a13f53 (HEAD-> master, 
> origin/master, origin/HEAD)
> Author: Simon Kelley <simon at thekelleys.org.uk>
> Date:   Mon Sep 1 22:35:02 2025 +0100
>
>    Optimise tftp.
>

I wonder if the if() clause (next to last line below) is correct, or is 
in the right place.
That's currently line #699.

The concern is that everything in the

     if(taddr.s_addr==context->router.s_addr)

expression is independent from tmp in the enlosing for(;;) loop, and I 
cannot fathom how this would make sense.

I suppose a reasonable compiler can decide the first for(tmp=context; 
tmp; tmp=tmp->current) does not cause observable effects (unless 
undefined behavior) and just discard the for loop.

Either we want to look at just the context->router.s_addr, then we don't 
need the for(),
or the if() clause needs to look at something with "tmp"
to make sense for both lines.

Also, I am confused reading the undocumented fields of struct 
dhcp_context {} without reading more than the declaration - it has one 
"current" and one "next" pointer and we iterate through "current", which 
isn't very typical from just reading it, but done in many places inside 
the code so I presume the pointer fields are only named confusingly, so 
I cannot assess what impact this inconsistency has.


> structdhcp_context*address_available(structdhcp_context*context,
> structin_addrtaddr,
> structdhcp_netid*netids)
> {
> /* Check is an address is OK for this network, check all
> possible ranges. Make sure that the address isn't in use
> by the server itself. */
> unsignedintstart, end, addr=ntohl(taddr.s_addr);
> structdhcp_context*tmp;
> for(tmp=context; tmp; tmp=tmp->current)
> if(taddr.s_addr==context->router.s_addr) // <- QUESTIONABLE for/if 
> VARIABLE CONCORD
> returnNULL;
> for(tmp=context; tmp; tmp=tmp->current)
> {
> start=ntohl(tmp->start.s_addr);
> end=ntohl(tmp->end.s_addr);
> if(!(tmp->flags&(CONTEXT_STATIC|CONTEXT_PROXY)) &&
> addr>=start&&
> addr<=end&&
> match_netid(tmp->filter, netids, 1))
> returntmp;
> }
> returnNULL;
> }

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/attachments/20250924/1b589f24/attachment-0001.htm>