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/
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel