Maxim Dounin
November 30, 2022 08:08PM
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@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@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/
_______________________________________________
nginx-devel mailing list -- nginx-devel@nginx.org
To unsubscribe send an email to nginx-devel-leave@nginx.org
Subject Author Views Posted

[PATCH] Removed the unsafe ngx_memcmp() wrapper for memcmp(3)

Alejandro Colomar 521 November 04, 2022 11:26AM

Re: [PATCH] Removed the unsafe ngx_memcmp() wrapper for memcmp(3)

Alejandro Colomar 119 November 04, 2022 11:30AM

Re: [PATCH] Removed the unsafe ngx_memcmp() wrapper for memcmp(3)

Maxim Dounin 123 November 04, 2022 10:40PM

Re: [PATCH] Removed the unsafe ngx_memcmp() wrapper for memcmp(3)

Alejandro Colomar 108 November 06, 2022 05:52PM

Re: [PATCH] Removed the unsafe ngx_memcmp() wrapper for memcmp(3)

Maxim Dounin 106 November 08, 2022 04:52AM

Re: [PATCH] Removed the unsafe ngx_memcmp() wrapper for memcmp(3)

Alejandro Colomar 150 November 08, 2022 05:50AM

[PATCH v2] Removed the casts within ngx_memcmp()

Alejandro Colomar 100 November 08, 2022 05:58AM

Re: [PATCH v2] Removed the casts within ngx_memcmp()

Maxim Dounin 100 November 09, 2022 10:04AM

Re: [PATCH v2] Removed the casts within ngx_memcmp()

Maxim Dounin 116 November 29, 2022 11:24PM

Re: [PATCH v2] Removed the casts within ngx_memcmp()

Alex Colomar 88 November 30, 2022 05:50AM

Re: [PATCH v2] Removed the casts within ngx_memcmp()

Maxim Dounin 135 November 30, 2022 08:22PM

Re: [PATCH v2] Removed the casts within ngx_memcmp()

Sergey Kandaurov 114 November 30, 2022 07:18AM

Re: [PATCH v2] Removed the casts within ngx_memcmp()

Maxim Dounin 90 November 30, 2022 08:08PM

u_char vs char (was: [PATCH] Removed the unsafe ngx_memcmp() wrapper for memcmp(3))

Alejandro Colomar 101 November 08, 2022 06:18AM



Sorry, you do not have permission to post/reply in this forum.

Online Users

Guests: 193
Record Number of Users: 8 on April 13, 2023
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready