[PATCH v2] Removed the casts within ngx_memcmp()

[Maxim Dounin]

Hello!

On Wed, Nov 30, 2022 at 04:17:15PM +0400, Sergey Kandaurov wrote:

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

Pushed to http://mdounin.ru/hg/nginx, thanks.

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

>From unix-history-repo it looks like there was no memcmp() in 
string.h in 4.3BSD-Tahoe release, and memcmp() was instead 
available via memory.h, without any prototypes:

https://github.com/dspinellis/unix-history-repo/blob/BSD-4_3_Tahoe/usr/src/include/string.h
https://github.com/dspinellis/unix-history-repo/blob/BSD-4_3_Tahoe/usr/src/include/memory.h

And in 4.3BSD-Reno release it was already using "const void *".

https://github.com/dspinellis/unix-history-repo/blob/BSD-4_3_Reno/usr/src/include/string.h

Looks like the variant with "const char *" prototypes available 
via string.h was only present shortly between these two versions.

This seems to match CSRG archive CDs from Kirk McKusick.

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

It does not look like MSVC 6.0 can be downloaded from Microsoft 
now, though CD copies are available at least at:

https://winworldpc.com/product/visual-c/6x
https://archive.org/details/microsoft-visual-c-6.0-standard-edition

And memcmp() there is defined as follows:

$ grep memcmp string.h
_CRTIMP int     __cdecl memcmp(const void *, const void *, size_t);
        int     __cdecl memcmp(const void *, const void *, size_t);

Though I'm pretty sure we don't care about anything older than 
MSVC2005 (which is still can be downloaded from Microsoft, BTW).

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