xref: /spdk/scripts/check_format.sh (revision 7506a7aa53d239f533af3bc768f0d2af55e735fe)
1#!/usr/bin/env bash
2
3if [[ $(uname -s) == Darwin ]]; then
4	# SPDK is not supported on MacOS, but as a developer
5	# convenience we support running the check_format.sh
6	# script on MacOS.
7	# Running "brew install bash greadlink ggrep" should be
8	# sufficient to get the correct versions of these utilities.
9	if [[ $(type -t mapfile) != builtin ]]; then
10		# We need bash version >= 4.0 for mapfile builtin
11		echo "Please install bash version >= 4.0"
12		exit 1
13	fi
14	if ! hash greadlink 2> /dev/null; then
15		# We need GNU readlink for -f option
16		echo "Please install GNU readlink"
17		exit 1
18	fi
19	if ! hash ggrep 2> /dev/null; then
20		# We need GNU grep for -P option
21		echo "Please install GNU grep"
22		exit 1
23	fi
24	GNU_READLINK="greadlink"
25	GNU_GREP="ggrep"
26else
27	GNU_READLINK="readlink"
28	GNU_GREP="grep"
29fi
30
31rootdir=$($GNU_READLINK -f "$(dirname "$0")")/..
32source "$rootdir/scripts/common.sh"
33
34cd "$rootdir"
35
36# exit on errors
37set -e
38
39if ! hash nproc 2> /dev/null; then
40
41	function nproc() {
42		echo 8
43	}
44
45fi
46
47function version_lt() {
48	[ $(echo -e "$1\n$2" | sort -V | head -1) != "$1" ]
49}
50
51function array_contains_string() {
52	name="$1[@]"
53	array=("${!name}")
54
55	for element in "${array[@]}"; do
56		if [ "$element" = "$2" ]; then
57			return $(true)
58		fi
59	done
60
61	return $(false)
62}
63
64rc=0
65
66function check_permissions() {
67	echo -n "Checking file permissions..."
68
69	local rc=0
70
71	while read -r perm _res0 _res1 path; do
72		if [ ! -f "$path" ]; then
73			continue
74		fi
75
76		# Skip symlinks
77		if [[ -L $path ]]; then
78			continue
79		fi
80		fname=$(basename -- "$path")
81
82		case ${fname##*.} in
83			c | h | cpp | cc | cxx | hh | hpp | md | html | js | json | svg | Doxyfile | yml | LICENSE | README | conf | in | Makefile | mk | gitignore | go | txt)
84				# These file types should never be executable
85				if [ "$perm" -eq 100755 ]; then
86					echo "ERROR: $path is marked executable but is a code file."
87					rc=1
88				fi
89				;;
90			*)
91				shebang=$(head -n 1 $path | cut -c1-3)
92
93				# git only tracks the execute bit, so will only ever return 755 or 644 as the permission.
94				if [ "$perm" -eq 100755 ]; then
95					# If the file has execute permission, it should start with a shebang.
96					if [ "$shebang" != "#!/" ]; then
97						echo "ERROR: $path is marked executable but does not start with a shebang."
98						rc=1
99					fi
100				else
101					# If the file does not have execute permissions, it should not start with a shebang.
102					if [ "$shebang" = "#!/" ]; then
103						echo "ERROR: $path is not marked executable but starts with a shebang."
104						rc=1
105					fi
106				fi
107				;;
108		esac
109
110	done <<< "$(git grep -I --name-only --untracked -e . | git ls-files -s)"
111
112	if [ $rc -eq 0 ]; then
113		echo " OK"
114	fi
115
116	return $rc
117}
118
119function check_c_style() {
120	local rc=0
121
122	if hash astyle; then
123		echo -n "Checking coding style..."
124		if [ "$(astyle -V)" \< "Artistic Style Version 3" ]; then
125			echo -n " Your astyle version is too old so skipping coding style checks. Please update astyle to at least 3.0.1 version..."
126		else
127			rm -f astyle.log
128			touch astyle.log
129			# Exclude rte_vhost code imported from DPDK - we want to keep the original code
130			#  as-is to enable ongoing work to synch with a generic upstream DPDK vhost library,
131			#  rather than making diffs more complicated by a lot of changes to follow SPDK
132			#  coding standards.
133			git ls-files '*.[ch]' '*.cpp' '*.cc' '*.cxx' '*.hh' '*.hpp' \
134				| grep -v rte_vhost | grep -v cpp_headers \
135				| xargs -P$(nproc) -n10 astyle --options=.astylerc >> astyle.log
136			if grep -q "^Formatted" astyle.log; then
137				echo " errors detected"
138				git diff --ignore-submodules=all
139				sed -i -e 's/  / /g' astyle.log
140				grep --color=auto "^Formatted.*" astyle.log
141				echo "Incorrect code style detected in one or more files."
142				echo "The files have been automatically formatted."
143				echo "Remember to add the files to your commit."
144				rc=1
145			else
146				echo " OK"
147			fi
148			rm -f astyle.log
149		fi
150	else
151		echo "You do not have astyle installed so your code style is not being checked!"
152	fi
153	return $rc
154}
155
156function check_comment_style() {
157	local rc=0
158
159	echo -n "Checking comment style..."
160
161	git grep --line-number -e '\/[*][^ *-]' -- '*.[ch]' > comment.log || true
162	git grep --line-number -e '[^ ][*]\/' -- '*.[ch]' ':!lib/rte_vhost*/*' >> comment.log || true
163	git grep --line-number -e '^[*]' -- '*.[ch]' >> comment.log || true
164	git grep --line-number -e '\s\/\/' -- '*.[ch]' >> comment.log || true
165	git grep --line-number -e '^\/\/' -- '*.[ch]' >> comment.log || true
166
167	if [ -s comment.log ]; then
168		echo " Incorrect comment formatting detected"
169		cat comment.log
170		rc=1
171	else
172		echo " OK"
173	fi
174	rm -f comment.log
175
176	return $rc
177}
178
179function check_spaces_before_tabs() {
180	local rc=0
181
182	echo -n "Checking for spaces before tabs..."
183	git grep --line-number $' \t' -- './*' ':!*.patch' > whitespace.log || true
184	if [ -s whitespace.log ]; then
185		echo " Spaces before tabs detected"
186		cat whitespace.log
187		rc=1
188	else
189		echo " OK"
190	fi
191	rm -f whitespace.log
192
193	return $rc
194}
195
196function check_trailing_whitespace() {
197	local rc=0
198
199	echo -n "Checking trailing whitespace in output strings..."
200
201	git grep --line-number -e ' \\n"' -- '*.[ch]' > whitespace.log || true
202
203	if [ -s whitespace.log ]; then
204		echo " Incorrect trailing whitespace detected"
205		cat whitespace.log
206		rc=1
207	else
208		echo " OK"
209	fi
210	rm -f whitespace.log
211
212	return $rc
213}
214
215function check_forbidden_functions() {
216	local rc=0
217
218	echo -n "Checking for use of forbidden library functions..."
219
220	git grep --line-number -w '\(atoi\|atol\|atoll\|strncpy\|strcpy\|strcat\|sprintf\|vsprintf\)' -- './*.c' ':!lib/rte_vhost*/**' > badfunc.log || true
221	if [ -s badfunc.log ]; then
222		echo " Forbidden library functions detected"
223		cat badfunc.log
224		rc=1
225	else
226		echo " OK"
227	fi
228	rm -f badfunc.log
229
230	return $rc
231}
232
233function check_cunit_style() {
234	local rc=0
235
236	echo -n "Checking for use of forbidden CUnit macros..."
237
238	git grep --line-number -w 'CU_ASSERT_FATAL' -- 'test/*' ':!test/spdk_cunit.h' > badcunit.log || true
239	if [ -s badcunit.log ]; then
240		echo " Forbidden CU_ASSERT_FATAL usage detected - use SPDK_CU_ASSERT_FATAL instead"
241		cat badcunit.log
242		rc=1
243	else
244		echo " OK"
245	fi
246	rm -f badcunit.log
247
248	return $rc
249}
250
251function check_eof() {
252	local rc=0
253
254	echo -n "Checking blank lines at end of file..."
255
256	if ! git grep -I -l -e . -z './*' ':!*.patch' \
257		| xargs -0 -P$(nproc) -n1 scripts/eofnl > eofnl.log; then
258		echo " Incorrect end-of-file formatting detected"
259		cat eofnl.log
260		rc=1
261	else
262		echo " OK"
263	fi
264	rm -f eofnl.log
265
266	return $rc
267}
268
269function check_posix_includes() {
270	local rc=0
271
272	echo -n "Checking for POSIX includes..."
273	git grep -I -i -f scripts/posix.txt -- './*' ':!include/spdk/stdinc.h' ':!include/linux/**' ':!lib/rte_vhost*/**' ':!scripts/posix.txt' ':!*.patch' > scripts/posix.log || true
274	if [ -s scripts/posix.log ]; then
275		echo "POSIX includes detected. Please include spdk/stdinc.h instead."
276		cat scripts/posix.log
277		rc=1
278	else
279		echo " OK"
280	fi
281	rm -f scripts/posix.log
282
283	return $rc
284}
285
286function check_naming_conventions() {
287	local rc=0
288
289	echo -n "Checking for proper function naming conventions..."
290	# commit_to_compare = HEAD - 1.
291	commit_to_compare="$(git log --pretty=oneline --skip=1 -n 1 | awk '{print $1}')"
292	failed_naming_conventions=false
293	changed_c_libs=()
294	declared_symbols=()
295
296	# Build an array of all the modified C libraries.
297	mapfile -t changed_c_libs < <(git diff --name-only HEAD $commit_to_compare -- lib/**/*.c module/**/*.c | xargs -r dirname | sort | uniq)
298	# Matching groups are 1. qualifiers / return type. 2. function name 3. argument list / comments and stuff after that.
299	# Capture just the names of newly added (or modified) function definitions.
300	mapfile -t declared_symbols < <(git diff -U0 $commit_to_compare HEAD -- include/spdk*/*.h | sed -En 's/(^[+].*)(spdk[a-z,A-Z,0-9,_]*)(\(.*)/\2/p')
301
302	for c_lib in "${changed_c_libs[@]}"; do
303		lib_map_file="mk/spdk_blank.map"
304		defined_symbols=()
305		removed_symbols=()
306		exported_symbols=()
307		if ls "$c_lib"/*.map &> /dev/null; then
308			lib_map_file="$(ls "$c_lib"/*.map)"
309		fi
310		# Matching groups are 1. leading +sign. 2, function name 3. argument list / anything after that.
311		# Capture just the names of newly added (or modified) functions that start with "spdk_"
312		mapfile -t defined_symbols < <(git diff -U0 $commit_to_compare HEAD -- $c_lib | sed -En 's/(^[+])(spdk[a-z,A-Z,0-9,_]*)(\(.*)/\2/p')
313		# Capture the names of removed symbols to catch edge cases where we just move definitions around.
314		mapfile -t removed_symbols < <(git diff -U0 $commit_to_compare HEAD -- $c_lib | sed -En 's/(^[-])(spdk[a-z,A-Z,0-9,_]*)(\(.*)/\2/p')
315		for symbol in "${removed_symbols[@]}"; do
316			for i in "${!defined_symbols[@]}"; do
317				if [[ ${defined_symbols[i]} = "$symbol" ]]; then
318					unset -v 'defined_symbols[i]'
319				fi
320			done
321		done
322		# It's possible that we just modified a functions arguments so unfortunately we can't just look at changed lines in this function.
323		# matching groups are 1. All leading whitespace 2. function name. Capture just the symbol name.
324		mapfile -t exported_symbols < <(sed -En 's/(^[[:space:]]*)(spdk[a-z,A-Z,0-9,_]*);/\2/p' < $lib_map_file)
325		for defined_symbol in "${defined_symbols[@]}"; do
326			# if the list of defined symbols is equal to the list of removed symbols, then we are left with a single empty element. skip it.
327			if [ "$defined_symbol" = '' ]; then
328				continue
329			fi
330			not_exported=true
331			not_declared=true
332			if array_contains_string exported_symbols $defined_symbol; then
333				not_exported=false
334			fi
335
336			if array_contains_string declared_symbols $defined_symbol; then
337				not_declared=false
338			fi
339
340			if $not_exported || $not_declared; then
341				if ! $failed_naming_conventions; then
342					echo " found naming convention errors."
343				fi
344				echo "function $defined_symbol starts with spdk_ which is reserved for public API functions."
345				echo "Please add this function to its corresponding map file and a public header or remove the spdk_ prefix."
346				failed_naming_conventions=true
347				rc=1
348			fi
349		done
350	done
351
352	if ! $failed_naming_conventions; then
353		echo " OK"
354	fi
355
356	return $rc
357}
358
359function check_include_style() {
360	local rc=0
361
362	echo -n "Checking #include style..."
363	git grep -I -i --line-number "#include <spdk/" -- '*.[ch]' > scripts/includes.log || true
364	if [ -s scripts/includes.log ]; then
365		echo "Incorrect #include syntax. #includes of spdk/ files should use quotes."
366		cat scripts/includes.log
367		rc=1
368	else
369		echo " OK"
370	fi
371	rm -f scripts/includes.log
372
373	return $rc
374}
375
376function check_python_style() {
377	local rc=0
378
379	if hash pycodestyle 2> /dev/null; then
380		PEP8=pycodestyle
381	elif hash pep8 2> /dev/null; then
382		PEP8=pep8
383	fi
384
385	if [ -n "${PEP8}" ]; then
386		echo -n "Checking Python style..."
387
388		PEP8_ARGS+=" --max-line-length=140"
389
390		error=0
391		git ls-files '*.py' | xargs -P$(nproc) -n1 $PEP8 $PEP8_ARGS > pep8.log || error=1
392		if [ $error -ne 0 ]; then
393			echo " Python formatting errors detected"
394			cat pep8.log
395			rc=1
396		else
397			echo " OK"
398		fi
399		rm -f pep8.log
400	else
401		echo "You do not have pycodestyle or pep8 installed so your Python style is not being checked!"
402	fi
403
404	return $rc
405}
406
407function get_bash_files() {
408	local sh shebang
409
410	mapfile -t sh < <(git ls-files '*.sh')
411	mapfile -t shebang < <(git grep -l '^#!.*bash')
412
413	printf '%s\n' "${sh[@]}" "${shebang[@]}" | sort -u
414}
415
416function check_bash_style() {
417	local rc=0
418
419	# find compatible shfmt binary
420	shfmt_bins=$(compgen -c | grep '^shfmt' | uniq || true)
421	for bin in $shfmt_bins; do
422		shfmt_version=$("$bin" --version)
423		if [ $shfmt_version != "v3.1.0" ]; then
424			echo "$bin version $shfmt_version not used (only v3.1.0 is supported)"
425			echo "v3.1.0 can be installed using 'scripts/pkgdep.sh -d'"
426		else
427			shfmt=$bin
428			break
429		fi
430	done
431
432	if [ -n "$shfmt" ]; then
433		shfmt_cmdline=() sh_files=()
434
435		mapfile -t sh_files < <(get_bash_files)
436
437		if ((${#sh_files[@]})); then
438			printf 'Checking .sh formatting style...'
439
440			shfmt_cmdline+=(-i 0)     # indent_style = tab|indent_size = 0
441			shfmt_cmdline+=(-bn)      # binary_next_line = true
442			shfmt_cmdline+=(-ci)      # switch_case_indent = true
443			shfmt_cmdline+=(-ln bash) # shell_variant = bash (default)
444			shfmt_cmdline+=(-d)       # diffOut - print diff of the changes and exit with != 0
445			shfmt_cmdline+=(-sr)      # redirect operators will be followed by a space
446
447			diff=${output_dir:-$PWD}/$shfmt.patch
448
449			# Explicitly tell shfmt to not look for .editorconfig. .editorconfig is also not looked up
450			# in case any formatting arguments has been passed on its cmdline.
451			if ! SHFMT_NO_EDITORCONFIG=true "$shfmt" "${shfmt_cmdline[@]}" "${sh_files[@]}" > "$diff"; then
452				# In case shfmt detects an actual syntax error it will write out a proper message on
453				# its stderr, hence the diff file should remain empty.
454				if [[ -s $diff ]]; then
455					diff_out=$(< "$diff")
456				fi
457
458				cat <<- ERROR_SHFMT
459
460					* Errors in style formatting have been detected.
461					${diff_out:+* Please, review the generated patch at $diff
462
463					# _START_OF_THE_DIFF
464
465					${diff_out:-ERROR}
466
467					# _END_OF_THE_DIFF
468					}
469
470				ERROR_SHFMT
471				rc=1
472			else
473				rm -f "$diff"
474				printf ' OK\n'
475			fi
476		fi
477	else
478		echo "Supported version of shfmt not detected, Bash style formatting check is skipped"
479	fi
480
481	return $rc
482}
483
484function check_bash_static_analysis() {
485	local rc=0
486
487	if hash shellcheck 2> /dev/null; then
488		echo -n "Checking Bash static analysis with shellcheck..."
489
490		shellcheck_v=$(shellcheck --version | grep -P "version: [0-9\.]+" | cut -d " " -f2)
491
492		# SHCK_EXCLUDE contains a list of all of the spellcheck errors found in SPDK scripts
493		# currently. New errors should only be added to this list if the cost of fixing them
494		# is deemed too high. For more information about the errors, go to:
495		# https://github.com/koalaman/shellcheck/wiki/Checks
496		# Error descriptions can also be found at: https://github.com/koalaman/shellcheck/wiki
497		# SPDK fails some error checks which have been deprecated in later versions of shellcheck.
498		# We will not try to fix these error checks, but instead just leave the error types here
499		# so that we can still run with older versions of shellcheck.
500		SHCK_EXCLUDE="SC1117"
501		# SPDK has decided to not fix violations of these errors.
502		# We are aware about below exclude list and we want this errors to be excluded.
503		# SC1083: This {/} is literal. Check expression (missing ;/\n?) or quote it.
504		# SC1090: Can't follow non-constant source. Use a directive to specify location.
505		# SC1091: Not following: (error message here)
506		# SC2001: See if you can use ${variable//search/replace} instead.
507		# SC2010: Don't use ls | grep. Use a glob or a for loop with a condition to allow non-alphanumeric filenames.
508		# SC2015: Note that A && B || C is not if-then-else. C may run when A is true.
509		# SC2016: Expressions don't expand in single quotes, use double quotes for that.
510		# SC2034: foo appears unused. Verify it or export it.
511		# SC2046: Quote this to prevent word splitting.
512		# SC2086: Double quote to prevent globbing and word splitting.
513		# SC2119: Use foo "$@" if function's $1 should mean script's $1.
514		# SC2120: foo references arguments, but none are ever passed.
515		# SC2128: Expanding an array without an index only gives the first element.
516		# SC2148: Add shebang to the top of your script.
517		# SC2153: Possible Misspelling: MYVARIABLE may not be assigned, but MY_VARIABLE is.
518		# SC2154: var is referenced but not assigned.
519		# SC2164: Use cd ... || exit in case cd fails.
520		# SC2174: When used with -p, -m only applies to the deepest directory.
521		# SC2178: Variable was used as an array but is now assigned a string.
522		# SC2206: Quote to prevent word splitting/globbing,
523		#         or split robustly with mapfile or read -a.
524		# SC2207: Prefer mapfile or read -a to split command output (or quote to avoid splitting).
525		# SC2223: This default assignment may cause DoS due to globbing. Quote it.
526		SHCK_EXCLUDE="$SHCK_EXCLUDE,SC1083,SC1090,SC1091,SC2010,SC2015,SC2016,SC2034,SC2046,SC2086,\
527SC2119,SC2120,SC2128,SC2148,SC2153,SC2154,SC2164,SC2174,SC2178,SC2001,SC2206,SC2207,SC2223"
528
529		SHCK_FORMAT="tty"
530		SHCK_APPLY=false
531		SHCH_ARGS="-e $SHCK_EXCLUDE -f $SHCK_FORMAT"
532
533		if ge "$shellcheck_v" 0.4.0; then
534			SHCH_ARGS+=" -x"
535		else
536			echo "shellcheck $shellcheck_v detected, recommended >= 0.4.0."
537		fi
538
539		get_bash_files | xargs -P$(nproc) -n1 shellcheck $SHCH_ARGS &> shellcheck.log
540		if [[ -s shellcheck.log ]]; then
541			echo " Bash shellcheck errors detected!"
542
543			cat shellcheck.log
544			if $SHCK_APPLY; then
545				git apply shellcheck.log
546				echo "Bash errors were automatically corrected."
547				echo "Please remember to add the changes to your commit."
548			fi
549			rc=1
550		else
551			echo " OK"
552		fi
553		rm -f shellcheck.log
554	else
555		echo "You do not have shellcheck installed so your Bash static analysis is not being performed!"
556	fi
557
558	return $rc
559}
560
561function check_changelog() {
562	local rc=0
563
564	# Check if any of the public interfaces were modified by this patch.
565	# Warn the user to consider updating the changelog any changes
566	# are detected.
567	echo -n "Checking whether CHANGELOG.md should be updated..."
568	staged=$(git diff --name-only --cached .)
569	working=$(git status -s --porcelain --ignore-submodules | grep -iv "??" | awk '{print $2}')
570	files="$staged $working"
571	if [[ "$files" = " " ]]; then
572		files=$(git diff-tree --no-commit-id --name-only -r HEAD)
573	fi
574
575	has_changelog=0
576	for f in $files; do
577		if [[ $f == CHANGELOG.md ]]; then
578			# The user has a changelog entry, so exit.
579			has_changelog=1
580			break
581		fi
582	done
583
584	needs_changelog=0
585	if [ $has_changelog -eq 0 ]; then
586		for f in $files; do
587			if [[ $f == include/spdk/* ]] || [[ $f == scripts/rpc.py ]] || [[ $f == etc/* ]]; then
588				echo ""
589				echo -n "$f was modified. Consider updating CHANGELOG.md."
590				needs_changelog=1
591			fi
592		done
593	fi
594
595	if [ $needs_changelog -eq 0 ]; then
596		echo " OK"
597	else
598		echo ""
599	fi
600
601	return $rc
602}
603
604function check_json_rpc() {
605	local rc=0
606
607	echo -n "Checking that all RPCs are documented..."
608	while IFS='"' read -r _ rpc _; do
609		if ! grep -q "^### $rpc" doc/jsonrpc.md; then
610			echo "Missing JSON-RPC documentation for ${rpc}"
611			rc=1
612		fi
613	done < <(git grep -h -E "^SPDK_RPC_REGISTER\(" ':!test/*')
614
615	if [ $rc -eq 0 ]; then
616		echo " OK"
617	fi
618	return $rc
619}
620
621function check_markdown_format() {
622	local rc=0
623
624	if hash mdl 2> /dev/null; then
625		echo -n "Checking markdown files format..."
626		mdl -g -s $rootdir/mdl_rules.rb . > mdl.log || true
627		if [ -s mdl.log ]; then
628			echo " Errors in .md files detected:"
629			cat mdl.log
630			rc=1
631		else
632			echo " OK"
633		fi
634		rm -f mdl.log
635	else
636		echo "You do not have markdownlint installed so .md files not being checked!"
637	fi
638
639	return $rc
640}
641
642function check_rpc_args() {
643	local rc=0
644
645	echo -n "Checking rpc.py argument option names..."
646	grep add_argument scripts/rpc.py | $GNU_GREP -oP "(?<=--)[a-z0-9\-\_]*(?=\')" | grep "_" > badargs.log
647
648	if [[ -s badargs.log ]]; then
649		echo "rpc.py arguments with underscores detected!"
650		cat badargs.log
651		echo "Please convert the underscores to dashes."
652		rc=1
653	else
654		echo " OK"
655	fi
656	rm -f badargs.log
657	return $rc
658}
659
660rc=0
661
662check_permissions || rc=1
663check_c_style || rc=1
664
665GIT_VERSION=$(git --version | cut -d' ' -f3)
666
667if version_lt "1.9.5" "${GIT_VERSION}"; then
668	# git <1.9.5 doesn't support pathspec magic exclude
669	echo " Your git version is too old to perform all tests. Please update git to at least 1.9.5 version..."
670	exit $rc
671fi
672
673check_comment_style || rc=1
674check_markdown_format || rc=1
675check_spaces_before_tabs || rc=1
676check_trailing_whitespace || rc=1
677check_forbidden_functions || rc=1
678check_cunit_style || rc=1
679check_eof || rc=1
680check_posix_includes || rc=1
681check_naming_conventions || rc=1
682check_include_style || rc=1
683check_python_style || rc=1
684check_bash_style || rc=1
685check_bash_static_analysis || rc=1
686check_changelog || rc=1
687check_json_rpc || rc=1
688check_rpc_args || rc=1
689
690exit $rc
691