Is free(NULL) OK?

[Joel Sherrill]

On Sun, Jan 12, 2025, 5:21 AM James Browning via devel <devel at ntpsec.org>
wrote:

> On Saturday, January 11, 2025 10:43:47 PM Pacific Standard Time Hal Murray
> via
> devel wrote:
> > The Linux man page says
> >        If ptr is NULL, no operation is performed.
> >
> > I'd expect a wrapper if there really was a problem.
> >
> > This code is in ntpd/ntp_config.c
> >         if (NULL == s) {                        /* free() hates NULL */
> >                 s = estrdup("");
> >         }
> >
> > Any reason not to nuke that chunk of code?
>
> Have we looked beyond the sound bite? A bush league check by this armchair
> asshole suggest that a NULL that slips through here can go into estrdup,
> strlen, get_logmask, stats_config, or other places I haven't found yet.
> Just be
> aware that sucker is a stopper; it would be prudent to check everything
> downstream.
>

If you don't want to pass a NULL into a libc or POSIX call, you can't
depend on any of them checking. There is an assumption that dereferencing a
NULL pointer will cause a fault so in the interest of performance (e.g.
micro optimization), the checks are often left out

Some prototypes in header files will specify that an argument cannot be
null. If you are lucky, that will catch some null cases at compile time.

This assumption is invalid in environments without an MMU.

In practice, you should check every return value and argument.

I've taken to recommending a set of precondition and post condition debug
asserts for most of what we write. Use project specific macros so you can
tweak the information and have options beyond what assert does.

But you have to have robust tests inside the project for this to catch all
the cases. Otherwise, you leave the checks and turn the action into
something appropriate to be fielded.


> Ah, it looks like estrdup and strlen at least would react poorly[1] to
> being
> fed a null from the wrapping code. huh.
>
> [1]
> https://stackoverflow.com/questions/5796103/strlen-not-checking-for-null


--joel

>
>
>
> _______________________________________________
> devel mailing list
> devel at ntpsec.org
> https://lists.ntpsec.org/mailman/listinfo/devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ntpsec.org/pipermail/devel/attachments/20250112/b8b42184/attachment.htm>