| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
This macro is similar to C++'s static_cast(). It allows a very-limited
set of casts to be performed, but rejects most casts. Basically, it
only allows converting from char to u_char and vice-versa, while
respecting the const qualifier.
/*
* SYNOPSIS
* type char_cast(type, string);
*
* ARGUMENTS
* type Type to which 'string' should be cast.
* string String that should be cast to 'type'. It should be a
* pointer, or a pointer to a pointer, to a character type,
* possibly const-qualified.
*
* DESCRIPTION
* This macro resembles C++'s static_cast(). It performs a cast,
* as if '(type) string' was written, but it performs additional
* checks that make sure the cast is limited to character types,
* that the input expression is also of a character type, and that
* it doesn't remove a const qualifier.
*
* It also makes it easier to find all casts easily with grep(1).
*
* CAVEATS
* It doesn't allow using 'volatile'. However, this isn't a big
* issue, since we don't use it with strings. If necessary, the
* macro can be improved to support volatile.
*
* EXAMPLES
* int *ip = &i;
* char *s = "foo";
* const char *cs = "bar";
*
* p = (char *) s; /* OK. Although unsafe, because it
* can be confused with the case in
* the next line. */
* p = (char *) cs; /* Very unsafe; it will probably
* result in Undefined Behaviour. */
*
* p = char_cast(char *, s); // OK.
* p = char_cast(char *, cs); // Compiler error. Cast not allowed.
*
* p = (const char *) s; // OK.
* p = (const char *) cs; // OK.
*
* p = char_cast(const char *, s); // OK.
* p = char_cast(const char *, cs); // OK.
*
* p = (int *) cs; /* Extremely unsafe; it will almost
* certainly result in Undefined
* Behaviour. */
* p = char_cast(int *, cs); // Compiler error. Cast not allowed.
*
* p = (char *) ip; /* Technically OK, but probably
* not what you want. */
* p = char_cast(char *, ip); // Compiler error. Cast not allowed.
*
* SEE ALSO
* _Generic(3)
*/
Acked-by: Andrew Clayton <a.clayton@nginx.com>
Signed-off-by: Alejandro Colomar <alx@nginx.com>
|
|
The new documentation for nxt_length() is:
/*
* SYNOPSIS
* size_t nxt_length(string-literal);
*
* ARGUMENTS
* string-literal
* String literal of which we want to measure its length.
*
* DESCRIPTION
* This macro measures the length (as strlen(3) would do) of a
* string literal.
*
* It is functionally-equivalent to 'sizeof(s) - 1', and in fact,
* it uses sizeof(), so it shouldn't be used with strings that
* aren't string literals.
*
* This macro is safe from most misuses, since it guarantees that
* the argument is an array. If the argument is a pointer instead
* of an array, it will generate a compiler warning:
* '-Wsizeof-pointer-div'. This will avoid most silly mistakes.
*
* EXAMPLES
* size_t len;
*
* len = sizeof(1 ? "a" : "b"); // 8; this is not what you want/
* len = nxt_length(1 ? "a" : "b"); // Compiler warning.
*
* SEE ALSO
* <https://stackoverflow.com/a/57537491>
*/
sizeof() should never be used to get the size of an array. It is
very unsafe, since arrays easily decay to pointers, and sizeof()
applied to a pointer gives false results that compile and produce
silent bugs.
It's better to use nxt_sizeof_array(), which implements sizeof()
division, which recent compilers warn when used with pointers.
This change would have avoided a bug that we almost introduced
recently by using:
nxt_str_set(&port, (r->tls ? "https://" : "http://"));
which in the macro expansion runs:
(&port)->length = nxt_length((r->tls ? : "https://" : "http://"));
which evaluates to:
port.length = sizeof(r->tls ? "https://" : "http://") - 1;
which evaluates to:
port.length = 8 - 1;
Of course, we didn't want a compile-time-constant 8 there, but
rather the length of the string.
The above bug is not obvious to the untrained eye, so let's show some
example programs that may give some more hints about the problem.
$ cat sizeof.c
#include <stdio.h>
int
main(void)
{
printf("%zu\n", sizeof("01"));
printf("%zu\n", sizeof("012"));
printf("%zu\n", sizeof(char *));
}
$ cc -Wall -Wextra sizeof.c
$ ./a.out
3
4
8
sizeof() returns the size in bytes of the array passed to it, which in
case of char strings, it is equivalent to the length of the string + 1
(for the terminating '\0').
However, arrays decay very easily in C, and they decay to a pointer to
the first element in the array. In case of strings, that is a 'char *'.
When sizeof() is given a pointer, it returns the size of the pointer,
which in most platforms is 8.
The ternary operator (?) performs default promotions (and other
nefarious stuff) that may surprise even the most experienced
programmers. It contrasts the __builtin_choose_expr() GCC builtin [1],
which performs almost equivalently, but without the unwanted effects of
the ternary operator.
[1]: <https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005fchoose_005fexpr>
$ cat ?.c
#include <stdio.h>
int
main(void)
{
printf("%zu\n", sizeof("01"));
printf("%zu\n", sizeof(__builtin_choose_expr(1, "01", "01")));
printf("%zu\n", sizeof(1 ? "01" : "01"));
printf("%zu\n", sizeof(char *));
}
$ cc -Wall -Wextra ?.c
$ ./a.out
3
3
8
8
In the above program, we can see how the ternary operator (?) decays
the array into a pointer, and makes it so that sizeof() will return a
constant 8.
As we can see, everything in the use of the macro would make it look
like it should work, but the combination of some seemingly-safe side
effects of various C features produces a completely unexpected bug.
The bug dissected here was originally found in our Review Board:
<https://rb.nginx.com/r/1113/#review4063>
even though it was not fully understood what was causing it.
Link: <https://stackoverflow.com/questions/37538/how-do-i-determine-the-size-of-my-array-in-c/57537491#57537491>
Cc: Andrew Clayton <a.clayton@nginx.com>
Signed-off-by: Alejandro Colomar <alx@nginx.com>
|
|
This macro is just sizeof(array), with compiler warnings for when the
input is not an array. nxt_nitems() will trigger -Wsizeof-pointer-div
when the input is not an array. The multiplication by 0 is to ignore
the result of nxt_nitems(), which we only want for the warning.
/*
* SYNOPSIS
* size_t sizeof_array(array);
*
* ARGUMENTS
* array Array to be sized.
*
* DESCRIPTION
* Measure the size of an array --equivalent to sizeof(array)--.
* This macro has the benefit that it triggers the warning
* '-Wsizeof-pointer-div' when the argument is a pointer instead
* of an array, which can cause subtle bugs, sometimes impossible
* to detect through tests.
*
* EXAMPLES
* int *p;
* int a[1];
* size_t sp, sa;
*
* sp = sizeof_array(p); // Warning: -Wsizeof-pointer-div
* sa = sizeof_array(a); // OK
*
* SEE ALSO
* <https://stackoverflow.com/questions/37538/how-do-i-determine-the-size-of-my-array-in-c/57537491#57537491>
*/
Signed-off-by: Alejandro Colomar <alx@nginx.com>
|
|
We had a mix of styles for declaring function-like macros:
Style A:
#define \
foo() \
do { \
... \
} while (0)
Style B:
#define foo() \
do { \
... \
} while (0)
We had a similar number of occurences of each style:
$ grep -rnI '^\w*(.*\\' | wc -l
244
$ grep -rn 'define.*(.*)' | wc -l
239
(Those regexes aren't perfect, but a very decent approximation.)
Real examples:
$ find src -type f | xargs sed -n '/^nxt_double_is_zero/,/^$/p'
nxt_double_is_zero(f) \
(fabs(f) <= FLT_EPSILON)
$ find src -type f | xargs sed -n '/define nxt_http_field_set/,/^$/p'
#define nxt_http_field_set(_field, _name, _value) \
do { \
(_field)->name_length = nxt_length(_name); \
(_field)->value_length = nxt_length(_value); \
(_field)->name = (u_char *) _name; \
(_field)->value = (u_char *) _value; \
} while (0)
I'd like to standardize on a single style for them, and IMO,
having the identifier in the same line as #define is a better
option for the following reasons:
- Programmers are used to `#define foo() ...` (readability).
- One less line of code.
- The program for finding them is really simple (see below).
function grep_ngx_func()
{
if (($# != 1)); then
>&2 echo "Usage: ${FUNCNAME[0]} <func>";
return 1;
fi;
find src -type f \
| grep '\.[ch]$' \
| xargs grep -l "$1" \
| sort \
| xargs pcregrep -Mn "(?s)^\$[\w\s*]+?^$1\(.*?^}";
find src -type f \
| grep '\.[ch]$' \
| xargs grep -l "$1" \
| sort \
| xargs pcregrep -Mn "(?s)define $1\(.*?^$" \
| sed -E '1s/^[^:]+:[0-9]+:/&\n\n/';
}
$ grep_ngx_func
Usage: grep_ngx_func <func>
$ grep_ngx_func nxt_http_field_set
src/nxt_http.h:98:
#define nxt_http_field_set(_field, _name, _value) \
do { \
(_field)->name_length = nxt_length(_name); \
(_field)->value_length = nxt_length(_value); \
(_field)->name = (u_char *) _name; \
(_field)->value = (u_char *) _value; \
} while (0)
$ grep_ngx_func nxt_sprintf
src/nxt_sprintf.c:56:
u_char * nxt_cdecl
nxt_sprintf(u_char *buf, u_char *end, const char *fmt, ...)
{
u_char *p;
va_list args;
va_start(args, fmt);
p = nxt_vsprintf(buf, end, fmt, args);
va_end(args);
return p;
}
................
Scripted change:
................
$ find src -type f \
| grep '\.[ch]$' \
| xargs sed -i '/define *\\$/{N;s/ *\\\n/ /;s/ //}'
|
|
When testing some configurations of compilers and OSes, I noticed
that clang(1) 13 on Debian caused a function to be compiled but
unused, and the compiler triggered a compile error.
To avoid that error, use __attribute__((__unused__)). Let's call
our wrapper NXT_MAYBE_UNUSED, since it describes itself more
precisely than the GCC attribute name. It's also the name that
C2x (likely C23) has given to the standard attribute, which is
[[maybe_unused]], so it's also likely to be more readable because
of that name being in ISO C.
|
|
|
|
This method requires as many iterations as there are set bits,
while the previous one has to shift up to the position of the
highest bit.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
