Coverity showed that the return value of `dst_key_gettime` was
unchecked in INITIALIZE_STATE. If DST_TIME_CREATED was not set we
would set the state to be initialized to a weird last changed time.
This would normally not happen because DST_TIME_CREATED is always
set. However, we would rather set the time to now (as the comment
also indicates) not match the creation time.
The comment on INITIALIZE_STATE also needs updating as we no
longer always initialize to HIDDEN.
Revert the change from ad03c22e976411cad743bc02746b803a2f119df7 as
further testing has shown that with hyper-threading disabled, named with
ISC rwlocks outperforms named with pthread rwlocks in cold cache testing
scenarios. Since building named with pthread rwlocks might still be a
better choice for some workloads, keep the compile-time option which
enables that.
Add two tests that checks that dynamic zones
can be updated and will be signed appropriately.
One zone covers an update with freeze/thaw, the
other covers an update through nsupdate.
When dnssec-policy was introduced, it implicitly set inline-signing.
But DNSSEC maintenance required either inline-signing to be enabled,
or a dynamic zone. In other words, not in all cases you want to
DNSSEC maintain your zone with inline-signing.
Change the behavior and determine whether inline-signing is
required: if the zone is dynamic, don't use inline-signing,
otherwise implicitly set it.
You can also explicitly set inline-signing to yes with dnssec-policy,
the restriction that both inline-signing and dnssec-policy cannot
be set at the same time is now lifted.
However, 'inline-signing no;' on a non-dynamic zone with a
dnssec-policy is not possible.
The yamlget.py file was changed in !3311 as part of making the
python code pylint and flake8 compliant. This omitted setting
'item' to 'item[key]' which caused the digdelv yaml tests to fail.
Also, the pretty printing is not really necessary, so remove
the "if key not in item; print error" logic.
Change 5332 renamed "dnssec-keys" configuration statement to the
more descriptive "trust-anchors". Not all occurrences in the
documentation had been updated.
All our MSVS Project files share the same intermediate directory. We
know that this doesn't cause any problems, so we can just disable the
detection in the project files.
Example of the warning:
warning MSB8028: The intermediate directory (.\Release\) contains files shared from another project (dnssectool.vcxproj). This can lead to incorrect clean and rebuild behavior.
There was a missing indirection for the pluginlist_cb_t *callback in the
declaration of the cfg_pluginlist_foreach function. Reported by MSVC as:
lib\isccfg\parser.c(4057): warning C4028: formal parameter 4 different from declaration
Due to a way the stdatomic.h shim is implemented on Windows, the MSVC
always things that the outside type is the largest - atomic_(u)int_fast64_t.
This can lead to false positives as this one:
lib\dns\adb.c(3678): warning C4477: 'fprintf' : format string '%u' requires an argument of type 'unsigned int', but variadic argument 2 has type 'unsigned __int64'
We workaround the issue by loading the value in a scoped local variable
with correct type first.
MSVC documentation states: "This warning can be caused when a pointer to
a const or volatile item is assigned to a pointer not declared as
pointing to const or volatile."
Unfortunately, this happens when we dynamically allocate and deallocate
block of atomic variables using isc_mem_get and isc_mem_put.
Couple of examples:
lib\isc\hp.c(134): warning C4090: 'function': different 'volatile' qualifiers [C:\builds\isc-projects\bind9\lib\isc\win32\libisc.vcxproj]
lib\isc\hp.c(144): warning C4090: 'function': different 'volatile' qualifiers [C:\builds\isc-projects\bind9\lib\isc\win32\libisc.vcxproj]
lib\isc\stats.c(55): warning C4090: 'function': different 'volatile' qualifiers [C:\builds\isc-projects\bind9\lib\isc\win32\libisc.vcxproj]
lib\isc\stats.c(87): warning C4090: 'function': different 'volatile' qualifiers [C:\builds\isc-projects\bind9\lib\isc\win32\libisc.vcxproj]
The InterlockedOr8() and InterlockedAnd8() first argument was cast
to (atomic_int_fast8_t) instead of (atomic_int_fast8_t *), this was
reported by MSVC as:
warning C4024: '_InterlockedOr8': different types for formal and actual parameter 1
warning C4024: '_InterlockedAnd8': different types for formal and actual parameter 1
Our vcxproj files set the WarningLevel to Level3, which is too verbose
for a code that needs to be portable. That basically leads to ignoring
all the errors that MSVC produces. This commits downgrades the
WarningLevel to Level1 and enables treating warnings as errors for
Release builds. For the Debug builds the WarningLevel got upgraded to
Level4, and treating warnings as errors is explicitly disabled.
We should eventually make the code clean of all MSVC warnings, but it's
a long way to go for Level4, so it's more reasonable to start at Level1.
For reference[1], these are the warning levels as described by MSVC
documentation:
* /W0 suppresses all warnings. It's equivalent to /w.
* /W1 displays level 1 (severe) warnings. /W1 is the default setting
in the command-line compiler.
* /W2 displays level 1 and level 2 (significant) warnings.
* /W3 displays level 1, level 2, and level 3 (production quality)
warnings. /W3 is the default setting in the IDE.
* /W4 displays level 1, level 2, and level 3 warnings, and all level 4
(informational) warnings that aren't off by default. We recommend
that you use this option to provide lint-like warnings. For a new
project, it may be best to use /W4 in all compilations. This option
helps ensure the fewest possible hard-to-find code defects.
* /Wall displays all warnings displayed by /W4 and all other warnings
that /W4 doesn't include — for example, warnings that are off by
default.
* /WX treats all compiler warnings as errors. For a new project, it
may be best to use /WX in all compilations; resolving all warnings
ensures the fewest possible hard-to-find code defects.
1. https://docs.microsoft.com/en-us/cpp/build/reference/compiler-option-warning-level?view=vs-2019
Windows BIND releases produced by GitLab CI are built from Git
repositories, not from release tarballs, which means the "srcid" file is
not present in the top source directory when MSBuild is invoked. This
causes the Git commit hash for such builds to be set to "unset_id".
Enable win32utils/Configure to try determining the commit hash for a
build by invoking Git on the build host if the "srcid" file is not
present (which is what its Unix counterpart does).
Our python code didn't adhere to any coding standard. In this commit, we add
flame8 (https://pypi.org/project/flake8/), and pylint (https://www.pylint.org/).
There's couple of exceptions:
- ans.py scripts are not checked, nor fixed as part of this MR
- pylint's missing-*-docstring and duplicate-code checks have
been disabled via .pylintrc
Both exceptions should be removed in due time.
