Browse Source

Basic C Code lint checker and shell checker (#859)

* Factor build packages out into a more maintainable list

* Create a location for scripts to live

* Provide a make target to return the source dir as close as reasonable to the original distributed state

* Add a code lint step, checking the coding style

* Change test harness as recommended by shellcheck

* Ensure we actually have the linter tool installed

* Use the correct directory for cmake to run the tests

* Adjust for the older uncrustify in the current github ubuntu-latest

* Make one file pass the linter

* Integrate the lint with the existing test workflow

* Add files with minimal changes needed to the linter

* Add more files with minimal changes needed to the linter

* Dont build binaries if we fail the lint test

* Update the phony targets with the lint steps

* Ensure the flake8 package is installed in the new lint workflow job

* Use the makefile to drive the packages needed to install for linting

* No need to add dependancies on lint, just rely on the workflow status to show failure

* Update the scripts dir README to reflect current assumptions

* Rename and briefly document the indent.sh script

* Fix the ignore to ignore the right Makefile

* Rename the test_harness script to make it clear it is a shell script

* Provide a master lint make target and add a shell script lint tool

* Elminate stray tabs

* Drop include/auth.h from linter - there are inconsistant results with function definitions when using the current uncrustify rules
pull/864/head
Hamish Coleman 3 years ago
committed by GitHub
parent
commit
80b33cd1a9
No known key found for this signature in database GPG Key ID: 4AEE18F83AFDEB23
  1. 39
      .github/workflows/tests.yml
  2. 7
      .gitignore
  3. 2
      CMakeLists.txt
  4. 59
      Makefile.in
  5. 8
      doc/Scripts.md
  6. 9
      scripts/README.md
  7. 60
      scripts/indent.sh
  8. 8
      scripts/test_harness.sh
  9. 3
      src/edge_utils_win32.c
  10. 3
      tools/tests-compress.c
  11. 3
      tools/tests-hashing.c
  12. 3
      tools/tests-transform.c
  13. 34
      uncrustify.cfg

39
.github/workflows/tests.yml

@ -12,15 +12,31 @@ jobs:
steps: steps:
- uses: actions/checkout@v2 - uses: actions/checkout@v2
- name: Install essential
run: |
sudo apt-get install flake8
- name: Run minimal test set - name: Run minimal test set
run: | run: |
./autogen.sh ./autogen.sh
./configure ./configure
make test make test
make lint.python
lint:
name: Code syntax
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: Make the makefiles
run: |
./autogen.sh
./configure
- name: Install essential
run: |
make build-dep
- name: Run the lint tools
run: |
make lint
test_linux: test_linux:
needs: smoketest needs: smoketest
@ -216,8 +232,9 @@ jobs:
uses: codecov/codecov-action@v2 uses: codecov/codecov-action@v2
package_dpkg: package_dpkg:
needs: test_linux
name: Package for Debian/Ubuntu name: Package for Debian/Ubuntu
needs:
- test_linux
runs-on: ubuntu-latest runs-on: ubuntu-latest
strategy: strategy:
fail-fast: true fail-fast: true
@ -260,8 +277,9 @@ jobs:
path: packages/debian/*.deb path: packages/debian/*.deb
package_rpm: package_rpm:
needs: test_linux
name: Package for Redhat/RPM name: Package for Redhat/RPM
needs:
- test_linux
runs-on: ubuntu-latest runs-on: ubuntu-latest
steps: steps:
@ -296,8 +314,9 @@ jobs:
path: rpmbuild/RPMS/x86_64/*.rpm path: rpmbuild/RPMS/x86_64/*.rpm
binaries_windows: binaries_windows:
needs: test_windows
name: Binaries for Windows (x86_64-pc-mingw64) name: Binaries for Windows (x86_64-pc-mingw64)
needs:
- test_windows
runs-on: windows-latest runs-on: windows-latest
steps: steps:
@ -321,8 +340,9 @@ jobs:
path: binaries path: binaries
binaries_macos: binaries_macos:
needs: test_macos
name: Binaries for MacOS (x86_64-apple-darwin) name: Binaries for MacOS (x86_64-apple-darwin)
needs:
- test_macos
runs-on: macos-latest runs-on: macos-latest
steps: steps:
@ -351,8 +371,9 @@ jobs:
path: binaries path: binaries
binaries_linux_crosscompile: binaries_linux_crosscompile:
needs: test_linux
name: Binaries for linux name: Binaries for linux
needs:
- test_linux
runs-on: ubuntu-latest runs-on: ubuntu-latest
strategy: strategy:
fail-fast: true fail-fast: true

7
.gitignore

@ -4,7 +4,7 @@
configure configure
configure.ac configure.ac
config.* config.*
./Makefile /Makefile
tools/Makefile tools/Makefile
autom4te.cache autom4te.cache
edge edge
@ -45,3 +45,8 @@ tests/*.out
*.gcda *.gcda
*.gcov *.gcov
coverage/ coverage/
# Files generated while running linting
*.indent
*.unc-backup.md5~
*.unc-backup~

2
CMakeLists.txt

@ -271,6 +271,6 @@ install(FILES ${PROJECT_BINARY_DIR}/doc/n2n.7.gz
# TODO: # TODO:
# - Add the right dependancy so that the tests binaries get built first # - Add the right dependancy so that the tests binaries get built first
enable_testing() enable_testing()
add_test(tests ${PROJECT_SOURCE_DIR}/tools/test_harness ${PROJECT_BINARY_DIR} ${PROJECT_SOURCE_DIR}/tests) add_test(tests ${PROJECT_SOURCE_DIR}/scripts/test_harness.sh ${PROJECT_BINARY_DIR} ${PROJECT_SOURCE_DIR}/tests)
endif(DEFINED UNIX) endif(DEFINED UNIX)

59
Makefile.in

@ -83,7 +83,28 @@ N2N_LIB=libn2n.a
N2N_OBJS=$(patsubst src/%.c, src/%.o, $(wildcard src/*.c)) N2N_OBJS=$(patsubst src/%.c, src/%.o, $(wildcard src/*.c))
N2N_DEPS=$(wildcard include/*.h) $(wildcard src/*.c) Makefile N2N_DEPS=$(wildcard include/*.h) $(wildcard src/*.c) Makefile
# As source files pass the linter, they can be added here (If all the source
# is passing the linter tests, this can be refactored)
LINT_CCODE=\
include/edge_utils_win32.h \
include/n2n_define.h \
include/pearson.h \
include/speck.h \
src/edge_utils_win32.c \
src/example_edge_embed_quick_edge_init.c \
src/header_encryption.c \
src/sn_selection.c \
src/transform_cc20.c \
src/tuntap_linux.c \
src/wire.c \
tools/tests-compress.c \
tools/tests-elliptic.c \
tools/tests-hashing.c \
tools/tests-transform.c \
tools/tests-wire.c \
export LDLIBS export LDLIBS
LDLIBS+=-ln2n LDLIBS+=-ln2n
LDLIBS+=@N2N_LIBS@ LDLIBS+=@N2N_LIBS@
@ -108,12 +129,26 @@ APPS+=example_sn_embed
DOCS=edge.8.gz supernode.1.gz n2n.7.gz DOCS=edge.8.gz supernode.1.gz n2n.7.gz
# This is the superset of all packages that might be needed during the build.
# Mostly of use in automated build systems.
BUILD_DEP:=\
autoconf \
build-essential \
flake8 \
gcovr \
libcap-dev \
libzstd-dev \
shellcheck \
uncrustify \
SUBDIRS+=tools SUBDIRS+=tools
COVERAGEDIR?=coverage COVERAGEDIR?=coverage
.PHONY: $(SUBDIRS) .PHONY: $(SUBDIRS)
.PHONY: steps build push all clean install test cover gcov build-dep .PHONY: steps build push all clean distclean install test cover gcov build-dep
.PHONY: lint lint.python lint.ccode lint.shell
all: $(APPS) $(DOCS) $(SUBDIRS) all: $(APPS) $(DOCS) $(SUBDIRS)
tools: $(N2N_LIB) tools: $(N2N_LIB)
@ -147,11 +182,19 @@ $(N2N_LIB): $(N2N_OBJS)
win32/n2n_win32.a: win32 win32/n2n_win32.a: win32
test: tools test: tools
tools/test_harness scripts/test_harness.sh
lint: lint.python lint.ccode lint.shell
lint.python: lint.python:
flake8 scripts/n2nctl scripts/n2nhttpd flake8 scripts/n2nctl scripts/n2nhttpd
lint.ccode:
scripts/indent.sh $(LINT_CCODE)
lint.shell:
shellcheck scripts/*.sh
# To generate coverage information, run configure with # To generate coverage information, run configure with
# CFLAGS="-fprofile-arcs -ftest-coverage" LDFLAGS="--coverage" # CFLAGS="-fprofile-arcs -ftest-coverage" LDFLAGS="--coverage"
# and run the desired tests. Ensure that package gcovr is installed # and run the desired tests. Ensure that package gcovr is installed
@ -168,11 +211,10 @@ gcov:
gcov $(N2N_OBJS) gcov $(N2N_OBJS)
$(MAKE) -C tools gcov $(MAKE) -C tools gcov
# This is the superset of all packages that might be needed. # This is a convinent target to use during development or from a CI/CD system
# It is a convinent target to use during development or from a CI/CD system
build-dep: build-dep:
ifeq ($(CONFIG_TARGET),generic) ifeq ($(CONFIG_TARGET),generic)
sudo apt install build-essential autoconf libcap-dev libzstd-dev gcovr flake8 sudo apt install $(BUILD_DEP)
else ifeq ($(CONFIG_TARGET),darwin) else ifeq ($(CONFIG_TARGET),darwin)
brew install automake gcovr brew install automake gcovr
else else
@ -184,6 +226,13 @@ clean:
rm -f tests/*.out src/*.gcno src/*.gcda rm -f tests/*.out src/*.gcno src/*.gcda
for dir in $(SUBDIRS); do $(MAKE) -C $$dir clean; done for dir in $(SUBDIRS); do $(MAKE) -C $$dir clean; done
distclean:
rm -f tests/*.out src/*.gcno src/*.gcda src/*.indent src/*.unc-backup*
rm -rf autom4te.cache/
rm -f config.log config.status configure configure.ac Makefile tools/Makefile include/config.h include/config.h.in
rm -f doc/edge.8.gz doc/n2n.7.gz doc/supernode.1.gz
rm -f $(addprefix src/,$(APPS))
install: edge supernode edge.8.gz supernode.1.gz n2n.7.gz install: edge supernode edge.8.gz supernode.1.gz n2n.7.gz
echo "MANDIR=$(MANDIR)" echo "MANDIR=$(MANDIR)"
$(MKDIR) $(SBINDIR) $(MAN1DIR) $(MAN7DIR) $(MAN8DIR) $(MKDIR) $(SBINDIR) $(MAN1DIR) $(MAN7DIR) $(MAN8DIR)

8
doc/Scripts.md

@ -13,7 +13,13 @@ This shell script is used during development to help build on Windows
systems. An example of how to use it is shown in systems. An example of how to use it is shown in
the [Building document](Building.md) the [Building document](Building.md)
## `tools/test_harness` ## `scripts/indent.sh`
This shell script is a wrapper for the `uncrustify` C code style checker
which checks or applies a set of rules to the code. It is used during
the automated lint checks.
## `scripts/test_harness.sh`
This shell script is used to run automated tests during development. This shell script is used to run automated tests during development.

9
scripts/README.md

@ -0,0 +1,9 @@
This directory contains executables that are not compiled. Some of these may
end up installed for use by end users, but many of them are for use during
development, builds and tests.
Nothing in this directory should need compiling to use and they should be
written such that they do not need configuring (e.g: they might probe several
directories for their requirements)
See the [Scripts Documentation](../docs/Scripts.md) for further details

60
scripts/indent.sh

@ -0,0 +1,60 @@
#!/bin/sh
#
# Given one or more input source files, run a re-indenter on them.
help() {
echo "Usage: scripts/indent [-i] [file...]"
echo " -i modify file in place with reindent results"
echo ""
echo "By default, will output a diff and exitcode if changed are needed"
echo "If modifying files, no exit code or diff is output"
exit 1
}
[ -z "$1" ] && help
[ "$1" = "-h" ] && help
INPLACE=0
if [ "$1" = "-i" ]; then
shift
INPLACE=1
fi
## indentOneClang() {
## rm -f "$1.indent"
## clang-format "$1" >"$1.indent"
## if [ $? -ne 0 ]; then
## echo "Error while formatting \"$1\""
## RESULT=1
## return
## fi
## diff -u "$1" "$1.indent"
## if [ $? -ne 0 ]; then
## RESULT=1
## fi
## }
indentOne() {
IFILE="$1"
if [ "$INPLACE" -eq 0 ]; then
OFILE="$1.indent"
rm -f "$OFILE"
else
OFILE="$1"
fi
if ! uncrustify -c uncrustify.cfg -f "$IFILE" -o "$OFILE"; then
echo "Error while formatting \"$1\""
RESULT=1
return
fi
if ! diff -u "$IFILE" "$OFILE"; then
RESULT=1
fi
}
RESULT=0
while [ -n "$1" ]; do
indentOne "$1"
shift
done
exit $RESULT

8
tools/test_harness → scripts/test_harness.sh

@ -20,11 +20,11 @@ TESTDATA=tests
# Confirm we have all the tools and data # Confirm we have all the tools and data
for i in $TESTS; do for i in $TESTS; do
if [ ! -e $TOOLSDIR/$i ]; then if [ ! -e "$TOOLSDIR/$i" ]; then
echo "Could not find test $TOOLSDIR/$i" echo "Could not find test $TOOLSDIR/$i"
exit 1 exit 1
fi fi
if [ ! -e $TESTDATA/$i.expected ]; then if [ ! -e "$TESTDATA/$i.expected" ]; then
echo "Could not find testdata $TESTDATA/$i.expected" echo "Could not find testdata $TESTDATA/$i.expected"
exit 1 exit 1
fi fi
@ -34,6 +34,6 @@ done
set -e set -e
for i in $TESTS; do for i in $TESTS; do
echo "$TOOLSDIR/$i >$TESTDATA/$i.out" echo "$TOOLSDIR/$i >$TESTDATA/$i.out"
$TOOLSDIR/$i >$TESTDATA/$i.out "$TOOLSDIR/$i" >"$TESTDATA/$i.out"
cmp $TESTDATA/$i.expected $TESTDATA/$i.out cmp "$TESTDATA/$i.expected" "$TESTDATA/$i.out"
done done

3
src/edge_utils_win32.c

@ -96,8 +96,7 @@ int get_best_interface_ip(n2n_edge_t * eee, dec_ip_str_t ip_addr){
traceEvent(TRACE_DEBUG, "Gateway: %s\n", pAdapter->GatewayList.IpAddress.String); traceEvent(TRACE_DEBUG, "Gateway: %s\n", pAdapter->GatewayList.IpAddress.String);
strncpy(ip_addr, pAdapter->IpAddressList.IpAddress.String, sizeof(dec_ip_str_t)-1); strncpy(ip_addr, pAdapter->IpAddressList.IpAddress.String, sizeof(dec_ip_str_t)-1);
} }
} } else {
else {
traceEvent(TRACE_WARNING, "GetAdaptersInfo failed with error: %d\n", dwRetVal); traceEvent(TRACE_WARNING, "GetAdaptersInfo failed with error: %d\n", dwRetVal);
} }
if(pAdapterInfo != NULL) { if(pAdapterInfo != NULL) {

3
tools/tests-compress.c

@ -44,7 +44,8 @@ uint8_t PKT_CONTENT[]={
0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,
0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,
0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,
0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15 }; 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15
};
static void init_compression_for_benchmark (void) { static void init_compression_for_benchmark (void) {

3
tools/tests-hashing.c

@ -38,7 +38,8 @@ uint8_t PKT_CONTENT[]={
0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,
0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,
0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,
0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15 }; 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15
};
void test_pearson (void *buf, unsigned int bufsize) { void test_pearson (void *buf, unsigned int bufsize) {
char *test_name = "pearson"; char *test_name = "pearson";

3
tools/tests-transform.c

@ -41,7 +41,8 @@ uint8_t PKT_CONTENT[]={
0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,
0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,
0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,
0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15 }; 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15, 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15
};
/* Prototypes */ /* Prototypes */
static ssize_t do_encode_packet ( uint8_t * pktbuf, size_t bufsize, const n2n_community_t c ); static ssize_t do_encode_packet ( uint8_t * pktbuf, size_t bufsize, const n2n_community_t c );

34
uncrustify.cfg

@ -0,0 +1,34 @@
# Initial rules taken from a quick discussion
# (See https://github.com/ntop/n2n/commit/00159d0d012c6836fd972af1748833eeaf50fa22#commitcomment-57137247)
# 4 space indention (never use tabs)
indent_columns = 4
indent_with_tabs = 0
indent_switch_case = 4
# space between name and bracket during function define
sp_func_def_paren = force
sp_func_proto_paren = force
# no space between name and bracket during call
sp_func_call_paren = remove
# no space after if and while
sp_before_sparen = remove
#sp_while_paren_open = remove # only in newer uncrustify
# block-braces as seen above
nl_if_brace = remove
nl_brace_else = remove
nl_elseif_brace = remove
nl_else_brace = remove
#nl_before_opening_brace_func_class_def = remove # only in newer uncrustify
nl_for_brace = remove
nl_while_brace = remove
# multi-line parameters with indentation under the opening bracket
# looks like this is the default, but might be the following:
#indent_func_call_param = false ?
# Want to keep var definition alignment
#align_keep_extra_space = true
Loading…
Cancel
Save