Core: Avoid memcpy from NULL

Maxim Dounin mdounin at mdounin.ru
Thu Dec 21 00:35:12 UTC 2023


Hello!

On Sat, Dec 16, 2023 at 04:26:37PM -0500, Ben Kallus wrote:

> > In general macro definitions in nginx are used everywhere for
> > efficiency reasons
> 
> Clang inlines short functions with -O1, and GCC does so with -O2 or
> -O1 -finline-small-functions. Are there any platforms that Nginx needs
> to support for which short function inlining isn't sufficient to solve
> this issue?

In nginx, functions expected to be inlined are marked with 
"ngx_inline", which normally resolves to "inline" (on unix) or 
"__inline" (on win32).  As such, modern versions of both clang and 
gcc will inline corresponding functions unless optimization is 
disabled.

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

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

> > While some might prefer other approaches, writing code like
> > "ngx_memcpy(dst, src, ++len)" in nginx is just wrong, and
> > shouldn't be trusted to work, much like it won't work with
> > "ngx_cpymem(dst, src, ++len)".
> 
> It is indeed wrong to use an expression with a side-effect in an
> argument to cpymem, but it's also not very obvious that it's wrong. An
> inlined function solves the argument reevaluation issue without
> performance overhead.

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).

The point is: in nginx, it's anyway wrong to use arguments with 
side effects.  And even expressions without side effects might 
won't work.  While many macro definitions were adjusted to accept 
expressions instead of the bare variables (see 2f9214713666 and 
https://mailman.nginx.org/pipermail/nginx-devel/2020-July/013354.html 
for an example), some still don't or can be picky.  For example, 
good luck with doing something like "ngx_max(foo & 0xff, bar)".

As such, it's certainly not an argument against using checks in 
macro definitions in the particular patch.

-- 
Maxim Dounin
http://mdounin.ru/


More information about the nginx-devel mailing list