Coverity, const, free lesson please

[Hal Murray]

Coverity is now complaining that the NTS code in ntpd/ntp_config.c has 
storage leaks when it sets up the strings that configuring NTS uses.  The 
code is making a copy of a string from the config parser and storing it 
into a struct without freeing the old data.  But it's initially NULL and 
this only happens once so there is nothing to leak.  (But see below.)

So I say to myself, no big deal, I'll just toss in the free (9 of them) to 
keep Coverity happy.
But that generates a warning since those slots are defined as const char *.

So my first question is, how do I de-const something?  I tried 
free((void*)x), but then the compiler warns me about the cast dropping the 
const.

Is there a normal/clean way to free const stuff?

My next question is why is Coverity complaining about NTS configuring code 
and not all sorts of other places that are setting up strings?  My guess 
is that it might have something to do with NTS using a static struct 
rather than one that was just allocated.

Or maybe it's getting confused by the const?  I haven't checked the other 
places that setup strings.  That code might be old enough that many of the 
target slots aren't marked const.

Now for the interesting part.  I wonder how much of our code has actual 
storage leaks in this area.  All you have to do to trigger it is put 
multiple uses of the same named slot in your config file.  As far as I 
know, there is nothing in our yacc/bison code that says X can only occur 
once.  It might be nice to use the last one so you can put overrides in a 
userprovided file that gets included rather than editing the distro 
provided master file.

A few cases I checked use erealloc so maybe the old code is safe and it's 
only the new NTS code that got off on the the wrong foot -- it just does 
xxx = estrdup(yyy)

Old code (in ntp_util/stats_config() does this:
                leapfile_name = erealloc(leapfile_name, len + 1);
                memcpy(leapfile_name, value, len + 1);
But that's only one sample.

it looks like we need a new subroutine that does the erealloc and copy.
That would fix the new code and cleanup the old code.


-- 
These are my opinions.  I hate spam.