build: support `make lint-c` with clang-tidy

This supports linting of C code using clang-tidy to fix common
security and code quality issues early in the development workflow.
The benefit is that less time has to be spent in code reviews to
point out obvious problems, and ideally when the outstanding issues
are fixed, clang-tidy (and clang-format) can also be used to to
automatically fix basic problems and enforce common code style,
similarly to `go vet && go fmt` workflow.

diff --git a/.clang-tidy b/.clang-tidy
new file mode 100644 (file)
index 0000000..dafe048
--- /dev/null
+++ b/.clang-tidy
@@ -0,0 +1,11 @@
+---
+Checks: 'bugprone-*,cert-*,-clang-analyzer-unix.Malloc,-clang-analyzer-deadcode.DeadStores,-clang-analyzer-valist.Uninitialized,-readability-*,-readability-braces-*,-readability-else-after-return,google-readability-casting,misc-*,-misc-macro-parentheses,-misc-unused-parameters'
+WarningsAsErrors: 'cert-*,misc-*,readability-*,clang-analyzer-*,-readability-non-const-parameter'
+HeaderFilterRegex: 'contrib/ucw/*.h'
+CheckOptions:
+  - key:             readability-identifier-naming
+    value:           'lower_case'
+  - key:             readability-function-size.StatementThreshold
+    value:           '400'
+  - key:             readability-function-size.LineThreshold
+    value:           '500'
\ No newline at end of file

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 13bf2b06e5c4caa47e20be094b8a8e18d2963337..2509054f25f758da95ca892ea938a95ea1de5114 100644 (file)
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -26,9 +26,10 @@ build:linux:amd64:
 
 build:clang:linux:amd64:
   stage: build
+  image: $CI_REGISTRY/knot/knot-resolver/ci:debian-unstable  # newer Debian for newer Clang
   script:
-    - CXX=clang++ CC=clang PREFIX=$(pwd)/.local make -k all CFLAGS=-Werror
-    - CXX=clang++ CC=clang PREFIX=$(pwd)/.local make install CFLAGS=-Werror
+    - CXX=clang++-5.0 CC=clang-5.0 PREFIX=$(pwd)/.local make -k all CFLAGS=-Werror
+    - CXX=clang++-5.0 CC=clang-5.0 PREFIX=$(pwd)/.local make install CFLAGS=-Werror
   tags:
     - docker
     - linux
@@ -38,7 +39,16 @@ lint:lua:
   stage: test
   dependencies: []  # do not download build artifacts
   script:
-    - make lint
+    - make lint-lua
+  tags:
+    - docker
+
+lint:c:
+  stage: test
+  image: $CI_REGISTRY/knot/knot-resolver/ci:debian-unstable  # newer Debian for newer Clang
+  dependencies: []  # do not download build artifacts
+  script:
+    - make lint-c CLANG_TIDY="clang-tidy-5.0 -quiet"
   tags:
     - docker
 

diff --git a/Makefile b/Makefile
index 7812b21d217888437ee377764e9ad9f3cdf4d4dc..b47f68370b599d5187cc605f1b504e94b8b0f697 100644 (file)
--- a/Makefile
+++ b/Makefile
@@ -8,7 +8,9 @@ check: all tests
 clean: contrib-clean lib-clean daemon-clean client-clean modules-clean \
        tests-clean doc-clean bench-clean coverage-clean
 doc: doc-html
-lint: $(patsubst %.lua.in,%.lua,$(wildcard */*/*.lua.in))
+lint: lint-lua lint-c
+lint-c: libkres-lint kresd-lint kresc-lint
+lint-lua: $(patsubst %.lua.in,%.lua,$(wildcard */*/*.lua.in))
        luacheck --codes --formatter TAP .
 
 .PHONY: all install check clean doc info lint

diff --git a/ci/Dockerfile b/ci/Dockerfile
index 6c0427c73a5580bab4459d00699a48d8bcb9951b..5b3f53d3a77b713bba9ac5425204aae6ad954762 100644 (file)
--- a/ci/Dockerfile
+++ b/ci/Dockerfile
@@ -68,7 +68,4 @@ RUN apt-get install -y -qqq lcov
 RUN luarocks install luacov
 
 # LuaJIT binary for stand-alone scripting
-RUN apt-get install -y -qqq luajit
-
-# clang for kresd CI
-RUN apt-get install -y -qqq clang
+RUN apt-get install -y -qqq luajit
\ No newline at end of file

diff --git a/doc/build.rst b/doc/build.rst
index 0c9a9d033def05dc7fb8aa55dad9aa49dace5add..0032ce2bcdf76a3e46f12f014d632a74c2ce96c6 100644 (file)
--- a/doc/build.rst
+++ b/doc/build.rst
@@ -62,7 +62,8 @@ There are also *optional* packages that enable specific functionality in Knot DN
    "libprotobuf_ 3.0+", "``modules/dnstap``", "Protocol Buffers support for dnstap_."
    "`libprotobuf-c`_ 1.0+", "``modules/dnstap``", "C bindings for Protobuf."
    "libfstrm_ 0.2+", "``modules/dnstap``", "Frame Streams data transport protocol."
-   "luacheck_", "``lint``", "Syntax and static analysis checker for Lua."
+   "luacheck_", "``lint-lua``", "Syntax and static analysis checker for Lua."
+   "`clang-tidy`_", "``lint-c``", "Syntax and static analysis checker for C."
    "luacov_", "``check-config``", "Code coverage analysis for Lua modules."
 
 .. [#] Requires C99, ``__attribute__((cleanup))`` and ``-MMD -MP`` for dependency file generation. GCC, Clang and ICC are supported.
@@ -260,7 +261,7 @@ The `make coverage` target gathers both gcov code coverage for C files, and luac
 Running unit and integration tests
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-The linter requires luacheck_ and is executed by ``make lint``.
+The linter requires luacheck_ and `clang-tidy`_ and is executed by ``make lint``.
 The unit tests require cmocka_ and are executed by ``make check``.
 Tests for the dnstap module need go and are executed by ``make ckeck-dnstap``.
 
@@ -329,6 +330,7 @@ You can hack on the container by changing the container entrypoint to shell like
 .. _libprotobuf-c: https://github.com/protobuf-c/protobuf-c/wiki
 .. _libfstrm: https://github.com/farsightsec/fstrm
 .. _luacheck: http://luacheck.readthedocs.io
+.. _clang-tidy: http://clang.llvm.org/extra/clang-tidy/index.html
 .. _luacov: https://keplerproject.github.io/luacov/
 .. _lcov: http://ltp.sourceforge.net/coverage/lcov.php
 

diff --git a/platform.mk b/platform.mk
index 466e0c2e246ce88e3deb239af93526306a5355ab..f2657ed15326889adae88e6f4a74d1cfbc2e57cd 100644 (file)
--- a/platform.mk
+++ b/platform.mk
@@ -4,6 +4,7 @@
 
 # Platform-dependent stuff checks
 CCLD := $(CC)
+CLANG_TIDY ?= clang-tidy -quiet
 CGO := go tool cgo
 GO := go
 CAT := cat
@@ -101,6 +102,9 @@ else
        $(call quiet,CCLD,$$@) $$($(1)_CFLAGS) $(BUILD_CFLAGS) $$($(1)_OBJ) $(call SOVER,$(7),$(7),$(1)) -o $$@ $(4) $$($(1)_LIBS) $(BUILD_LDFLAGS) $$($(1)_LDFLAGS)
 endif
 endif
+# Linter rules
+$(1)-lint: $$($(1)_SOURCES)
+       $(call quiet,CLANG_TIDY,$(1)) $$($(1)_SOURCES) -- $(BUILD_CFLAGS) $$($(1)_CFLAGS) -DMP_FREELIST_SIZE=0 -D__clang_analyzer__
 # Additional rules
 $(1)-clean:
        $(RM) $$($(1)_OBJ) $$($(1)_DEP) $(2)/$(1)$(3)