[PATCH v2] Removed the casts within ngx_memcmp()

[Sergey Kandaurov]

On Wed, Nov 09, 2022 at 06:03:24PM +0300, Maxim Dounin wrote:
> # HG changeset patch
> # User Maxim Dounin <mdounin at mdounin.ru>
> # Date 1668004692 -10800
> #      Wed Nov 09 17:38:12 2022 +0300
> # Node ID fc79ea0724a92c1f463625a11ed4cb785cd342b7
> # Parent  42bc158a47ecb3c2bd0396c723c307c757f2770e
> Fixed alignment of ngx_memmove()/ngx_movemem() macro definitions.
> 
> diff --git a/src/core/ngx_string.h b/src/core/ngx_string.h
> --- a/src/core/ngx_string.h
> +++ b/src/core/ngx_string.h
> @@ -140,8 +140,8 @@ ngx_copy(u_char *dst, u_char *src, size_
>  #endif
>  
>  
> -#define ngx_memmove(dst, src, n)   (void) memmove(dst, src, n)
> -#define ngx_movemem(dst, src, n)   (((u_char *) memmove(dst, src, n)) + (n))
> +#define ngx_memmove(dst, src, n)  (void) memmove(dst, src, n)
> +#define ngx_movemem(dst, src, n)  (((u_char *) memmove(dst, src, n)) + (n))
>  
>  
>  /* msvc and icc7 compile memcmp() to the inline loop */
> # HG changeset patch
> # User Maxim Dounin <mdounin at mdounin.ru>
> # Date 1668005196 -10800
> #      Wed Nov 09 17:46:36 2022 +0300
> # Node ID 5269880f00df1e5ae08299165ec43435b759c5a3
> # Parent  fc79ea0724a92c1f463625a11ed4cb785cd342b7
> Removed casts from ngx_memcmp() macro.
> 
> Casts are believed to be not needed, since memcmp() has "const void *"
> arguments since introduction of the "void" type in C89.  And on pre-C89
> platforms nginx is unlikely to compile without warnings anyway, as there
> are no casts in memcpy() and memmove() calls.
> 
> These casts were added in 1648:89a47f19b9ec without any details on why they
> were added, and Igor does not remember details either.  The most plausible
> explanation is that they were copied from ngx_strcmp() and were not really
> needed even at that time.
> 
> Prodded by Alejandro Colomar.
> 
> diff --git a/src/core/ngx_string.h b/src/core/ngx_string.h
> --- a/src/core/ngx_string.h
> +++ b/src/core/ngx_string.h
> @@ -145,7 +145,7 @@ ngx_copy(u_char *dst, u_char *src, size_
>  
>  
>  /* msvc and icc7 compile memcmp() to the inline loop */
> -#define ngx_memcmp(s1, s2, n)  memcmp((const char *) s1, (const char *) s2, n)
> +#define ngx_memcmp(s1, s2, n)     memcmp(s1, s2, n)
>  
>  
>  u_char *ngx_cpystrn(u_char *dst, u_char *src, size_t n);
> 

Looks good.

Indeed, old memcmp definition is traced back to pre-ANSI.
In particular, you can find old implementation in 4.3BSD-Tahoe
(named as "Routines described in memory(BA_LIB); System V compatibility")
that uses "char *" arguments, until they were rewritten in 4.3BSD-Reno
in ANSI C by Chris Torek.
Also, it seems that VC 6.0 has memcmp with non-const void argument
as pre-C++98 (but I cannot support this clame with real facts).