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
