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/>
OpenPGP_signature
Description: OpenPGP digital signature
_______________________________________________ nginx-devel mailing list -- nginx-devel@nginx.org To unsubscribe send an email to nginx-devel-le...@nginx.org
