Core: Avoid memcpy from NULL

Ben Kallus benjamin.p.kallus.gr at dartmouth.edu
Fri Dec 29 16:50:36 UTC 2023


> Still, -O0 is often used at least during development, and it might
> be unreasonable to introduce extra function calls in basic
> primitives.

I don't think this is a major cause for concern. It is perfectly
reasonable for ngx_memcpy be a wrapper function around memcpy; I think
most people would assume that from the name. In fact, it's *already*
implemented as a function when NGX_MEMCPY_LIMIT is defined.

> Further, nginx generally supports all available platforms
> reasonably compatible with POSIX and C89.  This implies that
> inline might be not available.

On such platforms, moving ngx_memcpy to a function may introduce some
performance overhead. The question is whether slightly better
performance on obscure systems is worth the mental overhead of working
with function-like macros.

> Sure (but see above about performance overhead; and another
> question is if it needs to be solved, or following existing style
> is enough to never see the issue).

A little extra performance overhead on C89 systems, or builds with -O0
is a very small price to pay. ngx_resolver.c:4283 contains direct
evidence that function-like macros incur mental overhead:
```
ngx_memcpy(sin6->sin6_addr.s6_addr, addr6[j++].s6_addr, 16);
```
Here we have an expression with a side-effect being passed into a
function-like macro. As luck would have it, the second argument to
ngx_memcpy is evaluated only once, so this is coincidentally okay.
This particular landmine has been armed for a decade (see
https://github.com/nginx/nginx/blob/769eded73267274e018f460dd76b417538aa5934/src/core/ngx_resolver.c#L2902).
Thus, the existing style guidelines are not enough to prevent issues
with function-like macros from arising in nginx. Inline functions
solve this problem near-optimally.

> good luck with doing something like "ngx_max(foo & 0xff, bar)".

Nginx is littered with uses of expressions in ngx_max and ngx_min, it
just happens that none of those expressions use operators of lower
precedence than < and >. This seems like an invitation for human
error.

Thus, I argue that the best solution to the memcpy-from-NULL problem
is to replace ngx_memcpy and ngx_cpymem with inline functions with the
appropriate checks for n==0. Going forward, it's probably smart to
consider transitioning away from function-like macros more generally.

-Ben


More information about the nginx-devel mailing list