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 614 November 04, 2022 11:26AM

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

Alejandro Colomar 169 November 04, 2022 11:30AM

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

Maxim Dounin 170 November 04, 2022 10:40PM

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

Alejandro Colomar 158 November 06, 2022 05:52PM

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

Maxim Dounin 151 November 08, 2022 04:52AM

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

Alejandro Colomar 207 November 08, 2022 05:50AM

[PATCH v2] Removed the casts within ngx_memcmp()

Alejandro Colomar 145 November 08, 2022 05:58AM

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

Maxim Dounin 143 November 09, 2022 10:04AM

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

Maxim Dounin 193 November 29, 2022 11:24PM

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

Alex Colomar 135 November 30, 2022 05:50AM

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

Maxim Dounin 174 November 30, 2022 08:22PM

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

Sergey Kandaurov 169 November 30, 2022 07:18AM

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

Maxim Dounin 147 November 30, 2022 08:08PM

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

Alejandro Colomar 147 November 08, 2022 06:18AM



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

Online Users

Guests: 154
Record Number of Users: 8 on April 13, 2023
Record Number of Guests: 500 on July 15, 2024
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready