Double conf_init in case of config reload: is it needed?

[fzs]

mdnsd/src/mdnsd.c

Lines 432 to 440 in 4570d3e

	if (reload) {
	sys_init();
	for (iface = iface_iterator(1); iface; iface = iface_iterator(0)) {
	records_clear(iface->mdns);
	conf_init(iface, path, hostnm);
	}
	pidfile(PACKAGE_NAME);
	reload = 0;
	}

Why does this reload case do a sys_init (which runs conf_init on each interface), and then removes all records and calls a conf_init again on every interface? I am a bit confused as to why this is necessary.

Unfortunately neither the commit nor the PR introducing the records_clear has any information as to why removing all records is necessary. But if it is, then does this really need to happen in two loops over the interfaces? Wouldn't the records_clear rather have to be in sys_init or conf_init? It is initialization, after all (according to the name).

[fzs]

I am asking because I am looking into how to change the generation of A records to make it more flexible and fix issued when introducing IPv6 addresses.
If the clearing of the records is only needed in case of a reload, couldn't it be an argument to sys_init or even conf_init?

I am partially answering my question, because I forgot that sys_init will not run conf_init if nothing changed on the interfaces. But why is the conf_init necessary upon reload? It is not very clear to me.

[troglobit]

It's a while ago I did that code, so not really sure why. The end-goal, however, is to be able to handle both first startup and SIGHUP in the same manner. Most of my coaxing the daemon part (and mquery) has been to force it (them) to just do what I wanted without as few changes to libmdnsd as possible. So if you have ideas for improvements/rewrites/simplifications that still meet the that use-case go ahead and do it! Don't be afraid to do radical changes :)

[fzs]

Yeah, that's where the testing part comes in. :) I like to create some security by having test that test current behaviour before making major changes to parts I have no intimate familiarity with or limited understanding of. It is a bit tricky with the deamon, though.

[troglobit]

Sounds reasonable. I think at least parts of the daemon functionality is easier to verify with the component level testing I added initially. So I can help with that if you like?

[fzs]

Well, I thin we need to rework how A records are created, as it breaks when adding IPv6. So essentially I wanted to test that a valid A (and later AAAA) record is still created, after refactoring the code for that.
You created a base setup for the component test. How would I extend it in specific tests, e.g. adding IPv6 addresses? Just do it in the test script?

[troglobit]

Yes, the only test right now is discover.sh, which just checks the output from mquery. You could add an address.sh or similar that captures a pcap file (with tshark) and analyze it with tcpdump to check for IPv4 and IPv6 addresses in the mDNS packets.