Welcome! Log In Create A New Profile

Advanced

Re: Core: Avoid memcpy from NULL

This forum is currently read only. You can not log in or make any changes. This is a temporary situation.
Ben Kallus
December 29, 2023 11:52AM
> 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
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel
Subject Author Views Posted

Core: Avoid memcpy from NULL

Ben Kallus 689 December 13, 2023 11:10AM

Re: Core: Avoid memcpy from NULL

Maxim Dounin 154 December 14, 2023 09:44PM

Re: Core: Avoid memcpy from NULL

Dipl. Ing. Sergey Brester via nginx-devel 151 December 15, 2023 09:48AM

Re: Core: Avoid memcpy from NULL

Ben Kallus 123 December 15, 2023 11:30AM

Re: Core: Avoid memcpy from NULL

Maxim Dounin 138 December 15, 2023 11:18PM

Re: Core: Avoid memcpy from NULL

Ben Kallus 154 December 16, 2023 04:28PM

Re: Core: Avoid memcpy from NULL

Maxim Dounin 149 December 20, 2023 07:36PM

Re: Core: Avoid memcpy from NULL

Ben Kallus 127 December 29, 2023 11:52AM

Re: Core: Avoid memcpy from NULL

Maxim Dounin 151 December 31, 2023 02:54PM

Re: Core: Avoid memcpy from NULL

Ben Kallus 137 January 03, 2024 07:00PM

Re: Core: Avoid memcpy from NULL

Maxim Dounin 141 January 04, 2024 11:12AM

Re: Core: Avoid memcpy from NULL

Ben Kallus 135 January 09, 2024 11:20AM

Re: Core: Avoid memcpy from NULL

Maxim Dounin 119 January 09, 2024 02:26PM

Re: Core: Avoid memcpy from NULL

Ben Kallus 143 January 23, 2024 07:10PM

Re: Core: Avoid memcpy from NULL

Maxim Dounin 118 January 24, 2024 03:58AM

Re: Core: Avoid memcpy from NULL

karton 131 January 24, 2024 10:54PM



Online Users

Guests: 285
Record Number of Users: 8 on April 13, 2023
Record Number of Guests: 500 on July 15, 2024
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready