mdns stuff in intercept_finish is broken
Eric S. Raymond
esr at thyrsus.com
Mon Nov 30 13:39:53 UTC 2015
Hal Murray <hmurray at megapathdsl.net>:
>
> I think it only gets tested on NetBSD.
>
> The first problem is that it doesn't include the header file.
> The second is that mdns is over in ntpd.c
>
> I think the idea of moving that much actual code over there is a bad idea.
I reverted that change last night. You're right, it was a bad idea.
> This looks fishy :
> sig_desc = NULL;
> sig_desc = strsignal(sig);
> if (sig_desc == NULL)
> sig_desc = "";
>
> It's leftover from when strsignal might not exist and didn't get cleaned up
> when you removed that ifdef test. My man page says some systems (not Linux)
> might return NULL on an invalid signal. I assume anything passed to that
> routine will be valid.
Yes, I guess the "sig_desc = NULL;" can go
> There are 2 ways into that code. One is via a kill command, probably from
> something like "service ntpd stop". The other is from a SIGBUS. The SIGBUS
> case should probably bypass the cleanup and exit non-zero.
>
> I think the idea of doing more than a simple log message and exit is to free
> up all the mallocs so the tool that monitors malloc usage would see a close
> to clean result at exit and everything left would be lost memory. I don't
> see the code I expect. There is a lot of mallocing in the config file parser
> and big chunks of memory for the mrulist. The config stuff is using an
> atexit-hook. That's probably a bad idea if we are going to catch SIGBUS and
> expect to do anything.
You're almost certainly right about all this. Feel free to clean up
as you like; I can't get very excited about it, as I judge the
objective (free up all mallocs before exit) to be unachievable in
practice given NTP's level of allocation complexity and multiple exit
paths.
--
<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
More information about the devel
mailing list