diff options
author | Alejandro Colomar <alx@nginx.com> | 2022-11-07 11:27:47 +0100 |
---|---|---|
committer | Alejandro Colomar <alx@nginx.com> | 2022-11-15 13:05:24 +0100 |
commit | 6ab70192fa26e801ac5891aad85bc6b3ff819782 (patch) | |
tree | c7f6f4099fa602e2424bdfe20494a605e7af5b86 | |
parent | 269bc8e4e86b6be5cd7acd5ca41b1a86a9032c8c (diff) | |
download | unit-6ab70192fa26e801ac5891aad85bc6b3ff819782.tar.gz unit-6ab70192fa26e801ac5891aad85bc6b3ff819782.tar.bz2 |
Using nxt_sizeof_array() instead of sizeof() for strings (arrays).
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>
-rw-r--r-- | src/nxt_clang.h | 2 |
1 files changed, 1 insertions, 1 deletions
diff --git a/src/nxt_clang.h b/src/nxt_clang.h index 919d9168..f465d562 100644 --- a/src/nxt_clang.h +++ b/src/nxt_clang.h @@ -256,7 +256,7 @@ nxt_popcount(unsigned int x) #define nxt_length(s) \ - (sizeof(s) - 1) + (nxt_sizeof_array(s) - 1) #endif /* _NXT_CLANG_H_INCLUDED_ */ |