Formatting: "make fmt" with clang-format (#395)

Let's introduce an automatic code style formatting and checks for
Themis. We use clang-format and clang-tidy tools as they are generally
available and gives reasonable results.

Introduce two new targets for Makefile:

  - make fmt
  - make fmt_check

"fmt" is intended for humans to reformat the code before submitting
changes for review. Currently it formats only C code but it may be
extended to other languages as well.

"fmt_check" is intended for CI to do format checks to ensure that the
code is formatted accordingly. Unfortunately, clang-format does not
have a convenient check mode so we hack one with git diff.

clang-tidy requires the compilation flags so we integrate the format
checking into the build pipeline. It's a bit repetetive, but also
gives up a chance to cleanup the build system. And it uses Make
features nicely so we don't have to recheck and reformat the files
which are okay.

We start by formatting only Soter and Themis source code. Other parts
will be added later.

.clang-format

@@ -0,0 +1,19 @@
+---
+BasedOnStyle: LLVM
+AccessModifierOffset: -4
+AllowAllParametersOfDeclarationOnNextLine: false
+AllowShortBlocksOnASingleLine: false
+AllowShortCaseLabelsOnASingleLine: false
+AllowShortFunctionsOnASingleLine: false
+AllowShortIfStatementsOnASingleLine: false
+AllowShortLoopsOnASingleLine: false
+AlwaysBreakTemplateDeclarations: true
+BreakBeforeBraces: Linux
+BreakConstructorInitializersBeforeComma: true
+ColumnLimit: 100
+IndentWidth: 4
+ObjCSpaceAfterProperty: true
+PointerAlignment: Left
+TabWidth: 8
+UseTab: Never
+...

.clang-tidy

@@ -0,0 +1,4 @@
+---
+Checks: 'readability-*'
+FormatStyle: none
+...

Makefile

@@ -15,6 +15,8 @@
 #
 
 #CC = clang
+CLANG_FORMAT ?= clang-format
+CLANG_TIDY   ?= clang-tidy
 SHELL = /bin/bash
 SRC_PATH = src
 ifneq ($(BUILD_PATH),)
@@ -47,8 +49,8 @@ PRINT_ERROR = printf "$@ $(ERROR_STRING)\n" | $(AWK_CMD) && printf "$(CMD)\n$$LO
 PRINT_ERROR_ = printf "$(ERROR_STRING)\n" | $(AWK_CMD) && printf "$(CMD)\n$$LOG\n" && false
 PRINT_WARNING = printf "$@ $(WARN_STRING)\n" | $(AWK_CMD) && printf "$(CMD)\n$$LOG\n"
 PRINT_WARNING_ = printf "$(WARN_STRING)\n" | $(AWK_CMD) && printf "$(CMD)\n$$LOG\n"
-BUILD_CMD = LOG=$$($(CMD) 2>&1) ; if [ $$? -eq 1 ]; then $(PRINT_ERROR); elif [ "$$LOG" != "" ] ; then $(PRINT_WARNING); else $(PRINT_OK); fi;
-BUILD_CMD_ = LOG=$$($(CMD) 2>&1) ; if [ $$? -eq 1 ]; then $(PRINT_ERROR_); elif [ "$$LOG" != "" ] ; then $(PRINT_WARNING_); else $(PRINT_OK_); fi;
+BUILD_CMD = LOG=$$($(CMD) 2>&1) ; if [ $$? -ne 0 ]; then $(PRINT_ERROR); elif [ "$$LOG" != "" ] ; then $(PRINT_WARNING); else $(PRINT_OK); fi;
+BUILD_CMD_ = LOG=$$($(CMD) 2>&1) ; if [ $$? -ne 0 ]; then $(PRINT_ERROR_); elif [ "$$LOG" != "" ] ; then $(PRINT_WARNING_); else $(PRINT_OK_); fi;
 
 PKGINFO_PATH = PKGINFO
 
@@ -297,6 +299,25 @@ $(OBJ_PATH)/%.o: $(SRC_PATH)/%.c
 	@echo -n "compile "
 	@$(BUILD_CMD)
 
+FMT_FIXUP += $(THEMIS_FMT_FIXUP) $(SOTER_FMT_FIXUP)
+FMT_CHECK += $(THEMIS_FMT_CHECK) $(SOTER_FMT_CHECK)
+
+$(OBJ_PATH)/%.c.fmt_fixup $(OBJ_PATH)/%.h.fmt_fixup: \
+    CMD = $(CLANG_TIDY) -fix $< -- $(CFLAGS) 2>/dev/null && $(CLANG_FORMAT) -i $< && touch $@
+
+$(OBJ_PATH)/%.c.fmt_check $(OBJ_PATH)/%.h.fmt_check: \
+    CMD = $(CLANG_FORMAT) $< | diff -u $< - && $(CLANG_TIDY) $< -- $(CFLAGS) 2>/dev/null && touch $@
+
+$(OBJ_PATH)/%.fmt_fixup: $(SRC_PATH)/%
+	@mkdir -p $(@D)
+	@echo -n "fixup $< "
+	@$(BUILD_CMD_)
+
+$(OBJ_PATH)/%.fmt_check: $(SRC_PATH)/%
+	@mkdir -p $(@D)
+	@echo -n "check $< "
+	@$(BUILD_CMD_)
+
 #$(AUD_PATH)/%: CMD = $(CC) $(CFLAGS) -E -dI -dD $< -o $@
 $(AUD_PATH)/%: CMD = ./scripts/pp.sh  $< $@
 
@@ -324,6 +345,9 @@ include tools/afl/fuzzy.mk
 
 err: ; $(ERROR)
 
+fmt: $(FMT_FIXUP)
+fmt_check: $(FMT_CHECK)
+
 clean: CMD = rm -rf $(BIN_PATH)
 
 clean: nist_rng_test_suite_clean clean_rust

src/soter/boringssl/soter.mk

@@ -14,7 +14,8 @@
 # limitations under the License.
 #
 
-SOTER_SRC += $(wildcard $(CRYPTO_ENGINE)/*.c)
+CRYPTO_ENGINE_SOURCES += $(wildcard $(CRYPTO_ENGINE)/*.c)
+CRYPTO_ENGINE_HEADERS += $(wildcard $(CRYPTO_ENGINE)/*.h)
 
 # Put path to your OpenSSL/LibreSSL here
 OPENSSL_DIR = libs/librebin
@@ -39,4 +40,4 @@ $(BIN_PATH)/boringssl/crypto/libcrypto.a $(BIN_PATH)/boringssl/decrepit/libdecre
 	@echo "building embedded BoringSSL..."
 	@mkdir -p $(BIN_PATH)/boringssl
 	@cd $(BIN_PATH)/boringssl && cmake -DCMAKE_BUILD_TYPE=Release -DCMAKE_C_FLAGS="-fpic" ../../third_party/boringssl/src
-	@$(MAKE) -C $(BIN_PATH)/boringssl
+	@$(MAKE) -C $(BIN_PATH)/boringssl

src/soter/openssl/soter.mk

@@ -14,9 +14,9 @@
 # limitations under the License.
 #
 
-SOTER_SRC += $(wildcard $(CRYPTO_ENGINE)/*.c)
-SOTER_AUD_SRC += $(wildcard $(CRYPTO_ENGINE)/*.c)
-SOTER_AUD_SRC += $(wildcard $(CRYPTO_ENGINE)/*.h)
+CRYPTO_ENGINE_SOURCES += $(wildcard $(CRYPTO_ENGINE)/*.c)
+CRYPTO_ENGINE_HEADERS += $(wildcard $(CRYPTO_ENGINE)/*.h)
+
 # Put path to your OpenSSL/LibreSSL here
 OPENSSL_DIR = libs/librebin
 

src/soter/soter.mk

@@ -14,20 +14,29 @@
 # limitations under the License.
 #
 
-SOTER_SRC = $(wildcard $(SRC_PATH)/soter/*.c)
-SOTER_AUD_SRC = $(wildcard $(SRC_PATH)/soter/*.c)
-SOTER_AUD_SRC += $(wildcard $(SRC_PATH)/soter/*.h)
-# ed25519 we specify here, because it should be portable and does not depend on $(CRYPTO_ENGINE)
-SOTER_SRC += $(wildcard $(SRC_PATH)/soter/ed25519/*.c)
-SOTER_AUD_SRC += $(wildcard $(SRC_PATH)/soter/ed25519/*.c)
-SOTER_AUD_SRC += $(wildcard $(SRC_PATH)/soter/ed25519/*.h)
+SOTER_SOURCES = $(wildcard $(SRC_PATH)/soter/*.c)
+SOTER_HEADERS = $(wildcard $(SRC_PATH)/soter/*.h)
+ED25519_SOURCES = $(wildcard $(SRC_PATH)/soter/ed25519/*.c)
+ED25519_HEADERS = $(wildcard $(SRC_PATH)/soter/ed25519/*.h)
+
+SOTER_SRC = $(SOTER_SOURCES) $(ED25519_SOURCES) $(CRYPTO_ENGINE_SOURCES)
+
+SOTER_AUD_SRC += $(SOTER_SOURCES) $(ED25519_SOURCES) $(CRYPTO_ENGINE_SOURCES)
+SOTER_AUD_SRC += $(SOTER_HEADERS) $(ED25519_HEADERS) $(CRYPTO_ENGINE_HEADERS)
+
+# Ignore ed25519 during code reformatting as it is 3rd-party code (and it breaks clang-tidy)
+SOTER_FMT_SRC += $(SOTER_SOURCES) $(CRYPTO_ENGINE_SOURCES)
+SOTER_FMT_SRC += $(SOTER_HEADERS) $(CRYPTO_ENGINE_HEADERS)
 
 include $(CRYPTO_ENGINE)/soter.mk
 
 SOTER_OBJ = $(patsubst $(SRC_PATH)/%.c,$(OBJ_PATH)/%.o, $(SOTER_SRC))
 
 SOTER_AUD = $(patsubst $(SRC_PATH)/%,$(AUD_PATH)/%, $(SOTER_AUD_SRC))
 
+SOTER_FMT_FIXUP = $(patsubst $(SRC_PATH)/%,$(OBJ_PATH)/%.fmt_fixup,$(SOTER_FMT_SRC))
+SOTER_FMT_CHECK = $(patsubst $(SRC_PATH)/%,$(OBJ_PATH)/%.fmt_check,$(SOTER_FMT_SRC))
+
 SOTER_BIN = soter
 
 soter_pkgconfig:

src/themis/themis.mk

@@ -14,14 +14,20 @@
 # limitations under the License.
 #
 
-THEMIS_SRC = $(wildcard $(SRC_PATH)/themis/*.c)
-THEMIS_AUD_SRC = $(wildcard $(SRC_PATH)/themis/*.c)
-THEMIS_AUD_SRC += $(wildcard $(SRC_PATH)/themis/*.h)
+THEMIS_SOURCES = $(wildcard $(SRC_PATH)/themis/*.c)
+THEMIS_HEADERS = $(wildcard $(SRC_PATH)/themis/*.h)
+
+THEMIS_SRC = $(THEMIS_SOURCES)
+THEMIS_AUD_SRC = $(THEMIS_SOURCES) $(THEMIS_HEADERS)
+THEMIS_FMT_SRC = $(THEMIS_SOURCES) $(THEMIS_HEADERS)
 
 THEMIS_OBJ = $(patsubst $(SRC_PATH)/%.c,$(OBJ_PATH)/%.o, $(THEMIS_SRC))
 
 THEMIS_AUD = $(patsubst $(SRC_PATH)/%,$(AUD_PATH)/%, $(THEMIS_AUD_SRC))
 
+THEMIS_FMT_FIXUP = $(patsubst $(SRC_PATH)/%,$(OBJ_PATH)/%.fmt_fixup,$(THEMIS_FMT_SRC))
+THEMIS_FMT_CHECK = $(patsubst $(SRC_PATH)/%,$(OBJ_PATH)/%.fmt_check,$(THEMIS_FMT_SRC))
+
 THEMIS_BIN = themis
 
 themis_pkgconfig:

tests/soter/nist-sts/.clang-format

@@ -0,0 +1,4 @@
+---
+DisableFormat: true
+SortIncludes: false
+...