Hello!
On Wed, Nov 30, 2022 at 11:49:18AM +0100, Alex Colomar wrote:
> On 11/30/22 05:22, Maxim Dounin wrote:
> > Hello!
> >
> > Ping.
>
> Sorry, I didn't know you were waiting for my confirmation.
Not exactly yours. As per the project policy, patches need to be
approved by other core developers before being committed.
Other reviews are appreciated though, especially from the authors
of original submissions.
> >> # 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.
>
> Makes sense.
>
> >>
> >> 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.
>
> And of course, this patch LGTM :)
Thanks, committed and will be available in the next release.
Thanks for prodding this.
--
Maxim Dounin
http://mdounin.ru/
_______________________________________________
nginx-devel mailing list -- nginx-devel@nginx.org
To unsubscribe send an email to nginx-devel-leave@nginx.org