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