Skip to content

Commit

Permalink
Basic C Code lint checker and shell checker (#859)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
hamishcoleman authored Oct 23, 2021
1 parent ae502d9 commit 80b33cd
Show file tree
Hide file tree
Showing 26 changed files with 308 additions and 122 deletions.
39 changes: 30 additions & 9 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,31 @@ jobs:

steps:
- uses: actions/checkout@v2
- name: Install essential
run: |
sudo apt-get install flake8
- name: Run minimal test set
run: |
./autogen.sh
./configure
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:
needs: smoketest
Expand Down Expand Up @@ -216,8 +232,9 @@ jobs:
uses: codecov/codecov-action@v2

package_dpkg:
needs: test_linux
name: Package for Debian/Ubuntu
needs:
- test_linux
runs-on: ubuntu-latest
strategy:
fail-fast: true
Expand Down Expand Up @@ -260,8 +277,9 @@ jobs:
path: packages/debian/*.deb

package_rpm:
needs: test_linux
name: Package for Redhat/RPM
needs:
- test_linux
runs-on: ubuntu-latest

steps:
Expand Down Expand Up @@ -296,8 +314,9 @@ jobs:
path: rpmbuild/RPMS/x86_64/*.rpm

binaries_windows:
needs: test_windows
name: Binaries for Windows (x86_64-pc-mingw64)
needs:
- test_windows
runs-on: windows-latest

steps:
Expand All @@ -321,8 +340,9 @@ jobs:
path: binaries

binaries_macos:
needs: test_macos
name: Binaries for MacOS (x86_64-apple-darwin)
needs:
- test_macos
runs-on: macos-latest

steps:
Expand Down Expand Up @@ -351,8 +371,9 @@ jobs:
path: binaries

binaries_linux_crosscompile:
needs: test_linux
name: Binaries for linux
needs:
- test_linux
runs-on: ubuntu-latest
strategy:
fail-fast: true
Expand Down
7 changes: 6 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
configure
configure.ac
config.*
./Makefile
/Makefile
tools/Makefile
autom4te.cache
edge
Expand Down Expand Up @@ -45,3 +45,8 @@ tests/*.out
*.gcda
*.gcov
coverage/

# Files generated while running linting
*.indent
*.unc-backup.md5~
*.unc-backup~
2 changes: 1 addition & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,6 @@ install(FILES ${PROJECT_BINARY_DIR}/doc/n2n.7.gz
# TODO:
# - Add the right dependancy so that the tests binaries get built first
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)
59 changes: 54 additions & 5 deletions Makefile.in
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,28 @@ N2N_LIB=libn2n.a
N2N_OBJS=$(patsubst src/%.c, src/%.o, $(wildcard src/*.c))
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

LDLIBS+=-ln2n
LDLIBS+=@N2N_LIBS@

Expand All @@ -108,12 +129,26 @@ APPS+=example_sn_embed

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

COVERAGEDIR?=coverage

.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)

tools: $(N2N_LIB)
Expand Down Expand Up @@ -147,11 +182,19 @@ $(N2N_LIB): $(N2N_OBJS)
win32/n2n_win32.a: win32

test: tools
tools/test_harness
scripts/test_harness.sh

lint: lint.python lint.ccode lint.shell

lint.python:
flake8 scripts/n2nctl scripts/n2nhttpd

lint.ccode:
scripts/indent.sh $(LINT_CCODE)

lint.shell:
shellcheck scripts/*.sh

# To generate coverage information, run configure with
# CFLAGS="-fprofile-arcs -ftest-coverage" LDFLAGS="--coverage"
# and run the desired tests. Ensure that package gcovr is installed
Expand All @@ -168,11 +211,10 @@ gcov:
gcov $(N2N_OBJS)
$(MAKE) -C tools gcov

# This is the superset of all packages that might be needed.
# It is a convinent target to use during development or from a CI/CD system
# This is a convinent target to use during development or from a CI/CD system
build-dep:
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)
brew install automake gcovr
else
Expand All @@ -184,6 +226,13 @@ clean:
rm -f tests/*.out src/*.gcno src/*.gcda
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
echo "MANDIR=$(MANDIR)"
$(MKDIR) $(SBINDIR) $(MAN1DIR) $(MAN7DIR) $(MAN8DIR)
Expand Down
8 changes: 7 additions & 1 deletion doc/Scripts.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
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.

Expand Down
2 changes: 1 addition & 1 deletion include/auth.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ int bin_to_ascii (char *out, uint8_t *in, size_t in_len);

int ascii_to_bin (uint8_t *out, char *in);

int generate_private_key(n2n_private_public_key_t key, char *in);
int generate_private_key (n2n_private_public_key_t key, char *in);

int generate_public_key (n2n_private_public_key_t pub, n2n_private_public_key_t prv);

Expand Down
2 changes: 1 addition & 1 deletion include/edge_utils_win32.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ struct tunread_arg {
};

extern HANDLE startTunReadThread (struct tunread_arg *arg);
int get_best_interface_ip(n2n_edge_t * eee, dec_ip_str_t ip_addr);
int get_best_interface_ip (n2n_edge_t * eee, dec_ip_str_t ip_addr);


#endif /* WIN32 */
Expand Down
6 changes: 3 additions & 3 deletions include/n2n_define.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,14 +92,14 @@

/* Federation name and indicators */
#define FEDERATION_NAME "*Federation"
enum federation{IS_NO_FEDERATION = 0,IS_FEDERATION = 1};
enum federation {IS_NO_FEDERATION = 0,IS_FEDERATION = 1};

/* (un)purgeable community indicator (supernode) */
#define COMMUNITY_UNPURGEABLE 0
#define COMMUNITY_PURGEABLE 1

/* (un)purgeable supernode indicator */
enum sn_purge{SN_PURGEABLE = 0, SN_UNPURGEABLE = 1};
enum sn_purge {SN_PURGEABLE = 0, SN_UNPURGEABLE = 1};

/* Header encryption indicators */
#define HEADER_ENCRYPTION_UNKNOWN 0
Expand Down Expand Up @@ -133,7 +133,7 @@ enum n2n_mgmt_type {
/* which the socket explicitly is closed before reopening */

/* flag used in add_sn_to_list_by_mac_or_sock */
enum skip_add{SN_ADD = 0, SN_ADD_SKIP = 1, SN_ADD_ADDED = 2};
enum skip_add {SN_ADD = 0, SN_ADD_SKIP = 1, SN_ADD_ADDED = 2};

#define N2N_NETMASK_STR_SIZE 16 /* dotted decimal 12 numbers + 3 dots */
#define N2N_MACNAMSIZ 18 /* AA:BB:CC:DD:EE:FF + NULL*/
Expand Down
2 changes: 1 addition & 1 deletion include/pearson.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,4 @@ uint32_t pearson_hash_32 (const uint8_t *in, size_t len);

uint16_t pearson_hash_16 (const uint8_t *in, size_t len);

void pearson_hash_init();
void pearson_hash_init ();
10 changes: 5 additions & 5 deletions include/speck.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@

#define u512 __m512i

#define SPECK_ALIGNED_CTX 64
#define SPECK_ALIGNED_CTX 64

typedef struct {
u512 rk[34];
Expand All @@ -63,7 +63,7 @@ typedef struct {

#define u256 __m256i

#define SPECK_ALIGNED_CTX 32
#define SPECK_ALIGNED_CTX 32

typedef struct {
u256 rk[34];
Expand All @@ -79,8 +79,8 @@ typedef struct {

#define u128 __m128i

#define SPECK_ALIGNED_CTX 16
#define SPECK_CTX_BYVAL 1
#define SPECK_ALIGNED_CTX 16
#define SPECK_CTX_BYVAL 1

typedef struct {
u128 rk[34];
Expand Down Expand Up @@ -117,7 +117,7 @@ typedef struct {

int speck_ctr (unsigned char *out, const unsigned char *in, unsigned long long inlen,
const unsigned char *n,
speck_context_t *ctx);
speck_context_t *ctx);

int speck_init (speck_context_t **ctx, const unsigned char *k, int keysize);

Expand Down
9 changes: 9 additions & 0 deletions scripts/README.md
Original file line number Diff line number Diff line change
@@ -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 changes: 60 additions & 0 deletions scripts/indent.sh
Original file line number Diff line number Diff line change
@@ -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
Loading

0 comments on commit 80b33cd

Please sign in to comment.