Alejandro Colomar
November 08, 2022 05:58AM
From: Alejandro Colomar <alx@kernel.org>

The casts are unnecessary, since memcmp(3)'s arguments are
'const void *', which allows implicit conversion from any pointer type.
It might have been necessary in the times of K&R C, where 'void *'
didn't exist yet, up until the early 2000s, because some old systems
still had limited or no support for ISO C89. Those systems passed away
a long time ago, and current systems, even the oldest living ones, have
support for ISO C89.

The cast in this case is (almost) innocuous, because it only hides
warnings for conversions from integer to pointer such as:

nxt_memcmp(n, "foo", 3); // no warnings
memcmp(n, "foo", 3); // warning: integer to pointer conversion

which is a difficult bug to write, since it's too obvious. Such code
will probably be caught in a code review, but there's always a small
risk. Since there's no reason to keep the small risk around, when we
can just avoid it by removing the cast.

In general, it's better to avoid a cast if possible, since casts will
disable many compiler warnings regarding type safety.

Apart from the small risk, there's a bigger concern about readability.
Casts tend to disable compiler warnings, and so are unsafe by nature.
A lot of care needs to be taken when casts are used, and a programmer
reading the cast might wonder what was that specific cast trying to
achieve. The answer is clear: nothing. So it's better to just prevent
that moment of "wtf is this doing here".

Signed-off-by: Alejandro Colomar <alx@nginx.com>
---
src/core/ngx_string.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/core/ngx_string.h b/src/core/ngx_string.h
index 0fb9be72..6c218e22 100644
--- 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_t len)


/* 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);
--
2.37.2

_______________________________________________
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 99 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: 187
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