Welcome! Log In Create A New Profile

Advanced

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

Alejandro Colomar
November 08, 2022 05:50AM
Hi Maxim!

On 11/8/22 10:50, Maxim Dounin wrote:
[...]

>
> Another possibility is that these casts were simply added based on
> the ngx_str*() macro definitions and weren't actually needed.

Yeah, that was my most-likely guess. But wanted to consider other possibilities
just in case.

> Especially given that ngx_memcmp() was only used by ngx_memn2cmp()
> at that time, which is actually a string function.
>
>>> Similar casts are used by almost all nginx string macro
>>> definitions, since most C library functions accept "char *", while
>>> nginx uses "u_char *" for strings, and this results in useless
>>> warnings.
>>
>> That's true of str...(3) functions from <string.h>, but the mem...(3) functions
>> from <string.h> use 'void *', so conversions are automatic, and casts are
>> unnecessary (after ANSI/ISO C89; previously, K&R C did use 'char *').
>
> Sure. The point is: similar casts are used by ngx_str*() macro
> definitions, so it might be copied from there even if not really
> needed. And another point is: avoiding casts in ngx_memcmp()
> won't change anything, since there are lots of places with similar
> casts.

Yes, this one is (almost) a no-op change. However, I'll still send the patch,
since it's just a one-line patch, and more because of readability. Not
readability of the wrapper, which is as readable with or without the cast, but
about programmers trying to understand why the cast is there and what can it be
doing, since it actually does nothing. I think that's an improvement, even if
small in this case.

[...]

> Not really. We certainly do support Windows XP, but nginx also
> happened to work on HP-UX, AIX, and even on QNX at some point
> (though there were warnings due to unsigned time_t, and likely
> associated bugs). Some platforms are documented here:
>
> http://nginx.org/en/#tested_os_and_platforms
>
> And we certainly at least try to support anything reasonable /
> usable.
>
> On the other hand, it might be good enough to require -Wno-error
> or an equivalent on ancient platforms.

Yeah, that makes sense.

[...]

> I tend to agree that we can drop casts from the ngx_memcmp()
> macro.
>
> Note though that it's mostly nop change for the reasons given
> above.
>

So, I'll send the patch in a moment.

Cheers,

Alex

--
http://www.alejandro-colomar.es/
_______________________________________________
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 508 November 04, 2022 11:26AM

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

Alejandro Colomar 112 November 04, 2022 11:30AM

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

Maxim Dounin 115 November 04, 2022 10:40PM

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

Alejandro Colomar 101 November 06, 2022 05:52PM

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

Maxim Dounin 97 November 08, 2022 04:52AM

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

Alejandro Colomar 135 November 08, 2022 05:50AM

[PATCH v2] Removed the casts within ngx_memcmp()

Alejandro Colomar 92 November 08, 2022 05:58AM

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

Maxim Dounin 91 November 09, 2022 10:04AM

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

Maxim Dounin 108 November 29, 2022 11:24PM

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

Alex Colomar 79 November 30, 2022 05:50AM

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

Maxim Dounin 125 November 30, 2022 08:22PM

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

Sergey Kandaurov 108 November 30, 2022 07:18AM

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

Maxim Dounin 82 November 30, 2022 08:08PM

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

Alejandro Colomar 94 November 08, 2022 06:18AM



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

Online Users

Guests: 285
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