always print a colon after EDNS option names in YAML output
previously, if the option was empty, then it was printed without a
colon, which could not be parsed as YAML. adding a colon in all cases
addresses this problem.
Mark Andrews [Tue, 1 Oct 2019 04:06:53 +0000 (14:06 +1000)]
silence clang warning by using local variable.
'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.
Fix passing NULL after the last typed argument to a variadic function leads to undefined behaviour.
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.
// 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;
}
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.
The coccinellery repository provides many little semantic patches to fix common
problems in the code. The number of semantic patches in the coccinellery
repository is high and most of the semantic patches apply only for Linux, so it
doesn't make sense to run them on regular basis as the processing takes a lot of
time.
The list of issue found in BIND 9, by no means complete, includes:
- double assignment to a variable
- `continue` at the end of the loop
- double checks for `NULL`
- useless checks for `NULL` (cannot be `NULL`, because of earlier return)
- using `0` instead of `NULL`
- useless extra condition (`if (foo) return; if (!foo) { ...; }`)
- removing & in front of static functions passed as arguments
Split dns_name_copy() into dns_name_copy() and dns_name_copynf()
The dns_name_copy() function followed two different semanitcs that was driven
whether the last argument was or wasn't NULL. This commit splits the function
in two where now third argument to dns_name_copy() can't be NULL and
dns_name_copynf() doesn't have third argument.
The final round of adding RUNTIME_CHECK() around dns_name_copy() calls
This commit was done by hand to add the RUNTIME_CHECK() around stray
dns_name_copy() calls with NULL as third argument. This covers the edge cases
that doesn't make sense to write a semantic patch since the usage pattern was
unique or almost unique.
Add RUNTIME_CHECK() around result = dns_name_copy(..., NULL) calls
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.
Add RUNTIME_CHECK() around plain dns_name_copy(..., NULL) calls using spatch
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.
Add semantic patches to correctly check dns_name_copy(..., NULL) return code
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.
Test of valid A-label in locale that cannot display it only with non-broken idn2
The libidn2 library on Ubuntu Bionic is broken and idn2_to_unicode_8zlz() does't
fail when it should. This commit ensures that we don't run the system test for
valid A-label in locale that cannot display with the buggy libidn2 as it would
break the tests.
Petr Menšík [Tue, 29 Jan 2019 17:07:44 +0000 (18:07 +0100)]
Fallback to ASCII on output IDN conversion error
It is possible dig used ACE encoded name in locale, which does not
support converting it to unicode. Instead of fatal error, fallback to
ACE name on output.