Hi all + Ronan,
While reading through src/network/networkd-json.c, I noticed a comment
in dnr_append_json_one() that appears to have been left unresolved. Per
git blame, it looks like this line was introduced by Ronan Pigott on
2024-04-03 (in case you remember the context):
//FIXME ifindex?
return sd_json_variant_append_arrayb(
array,
SD_JSON_BUILD_OBJECT(
SD_JSON_BUILD_PAIR_INTEGER("Family", res->family),
...
The function dns_append_json_one() (used for plain DNS server entries)
includes a conditional InterfaceIndex field:
SD_JSON_BUILD_PAIR_CONDITION(a->ifindex != 0, "InterfaceIndex",
SD_JSON_BUILD_INTEGER(a->ifindex)),
but struct sd_dns_resolver (used for DNR entries) has no equivalent
ifindex field at all:
typedef struct sd_dns_resolver {
uint16_t priority;
char *auth_name;
int family;
union in_addr_union *addrs;
size_t n_addrs;
sd_dns_alpn_flags transports;
uint16_t port;
char *dohpath;
} sd_dns_resolver;
From what I can tell tracing through the codebase, sd_dns_resolver is
currently only ever parsed, tracked, and exposed read-only via JSON
(networkd-json.c).
Given that, I wanted to ask: is the missing ifindex field an intentional
deferral, or is it simply an oversight that never got revisited?
If it turns out a fix is wanted here, I will be happy to put together a
pull request on GitHub once I understand the intended direction -- just
didn't want to guess and open something that doesn't match the reasoning
behind the original comment.
Also, if a sd_dns_resolver is ever created elsewhere in the code, the
ifindex must be assigned properly or initialized to zero.
Either the comment can be removed or we can add a field to the
sd_dns_resolver so that interfaces can be included.
On top of all that - double slash comments aren't technically permitted
and this is how I found it.
Thanks,
Jeremy Dean
