This second commit uses second semantic patch to replace the calls to
dns_name_copy() with NULL as third argument where the result was stored in a
isc_result_t variable. As the dns_name_copy(..., NULL) cannot fail gracefully
when the third argument is NULL, it was just a bunch of dead code.
Couple of manual tweaks (removing dead labels and unused variables) were
manually applied on top of the semantic patch.
(cherry picked from commit 89b269b0d2)
This commit add RUNTIME_CHECK() around all simple dns_name_copy() calls where
the third argument is NULL using the semantic patch from the previous commit.
(cherry picked from commit 35bd7e4da0)
The dns_name_copy() function cannot fail gracefully when the last argument
(target) is NULL. Add RUNTIME_CHECK()s around such calls.
The first semantic patch adds RUNTIME_CHECK() around any call that ignores the
return value and is very safe to apply.
The second semantic patch attempts to properly add RUNTIME_CHECK() to places
where the return value from `dns_name_copy()` is recorded into `result`
variable. The result of this semantic patch needs to be reviewed by hand.
Both patches misses couple places where the code surrounding the
`dns_name_copy(..., NULL)` usage is more complicated and is better suited to be
fixed by a human being that understands the surrounding code.
(cherry picked from commit 406eba0c41)
'isc_commandline_index' is a global variable so it can theoretically
change result between if expressions. Save 'argv[isc_commandline_index]'
to local variable 'arg1' and use 'arg1 == NULL' in if expressions
instead of 'argc < isc_commandline_index + 1'. This allows clang
to correctly determine what code is reachable.
(cherry picked from commit 1b27ab8642)
From Cppcheck:
Passing NULL after the last typed argument to a variadic function leads to
undefined behaviour. The C99 standard, in section 7.15.1.1, states that if the
type used by va_arg() is not compatible with the type of the actual next
argument (as promoted according to the default argument promotions), the
behavior is undefined. The value of the NULL macro is an implementation-defined
null pointer constant (7.17), which can be any integer constant expression with
the value 0, or such an expression casted to (void*) (6.3.2.3). This includes
values like 0, 0L, or even 0LL.In practice on common architectures, this will
cause real crashes if sizeof(int) != sizeof(void*), and NULL is defined to 0 or
any other null pointer constant that promotes to int. To reproduce you might be
able to use this little code example on 64bit platforms. If the output includes
"ERROR", the sentinel had only 4 out of 8 bytes initialized to zero and was not
detected as the final argument to stop argument processing via
va_arg(). Changing the 0 to (void*)0 or 0L will make the "ERROR" output go away.
void f(char *s, ...) {
va_list ap;
va_start(ap,s);
for (;;) {
char *p = va_arg(ap,char*);
printf("%018p, %s\n", p, (long)p & 255 ? p : "");
if(!p) break;
}
va_end(ap);
}
void g() {
char *s2 = "x";
char *s3 = "ERROR";
// changing 0 to 0L for the 7th argument (which is intended to act as
// sentinel) makes the error go away on x86_64
f("first", s2, s2, s2, s2, s2, 0, s3, (char*)0);
}
void h() {
int i;
volatile unsigned char a[1000];
for (i = 0; i<sizeof(a); i++)
a[i] = -1;
}
int main() {
h();
g();
return 0;
}
(cherry picked from commit d8879af877)
This MR changes the default Debian sid build to wrap make with bear
that creates compilation database and use the compilation database
to run Cppcheck on the source files systematically.
The job is currently set to be allowed to fail as it will take some
time to fix all the Cppcheck detected issues.
(cherry picked from commit f55dc51f42)
Ondřej SurýandMark Andrews
Ondřej Surý
Evan Hunt