summaryrefslogtreecommitdiffstats
path: root/src/spdk/scripts/check_format.sh
blob: f4df5c41b174a0d9f8636a84188b2211271ef7e2 (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
376
377
378
379
380
381
382
383
384
385
386
387
388
389
390
391
392
393
394
395
396
397
398
399
400
401
402
403
404
405
406
407
408
409
410
411
412
413
414
415
416
417
418
419
420
421
422
423
424
425
426
427
428
429
430
431
432
433
434
435
436
437
438
439
440
441
442
443
444
445
446
447
448
449
450
451
452
453
454
455
456
457
458
459
460
461
462
463
464
465
466
467
468
469
470
471
472
473
474
475
476
477
478
479
480
481
482
483
484
485
486
487
488
489
490
491
492
493
494
495
496
497
498
499
500
#!/usr/bin/env bash

readonly BASEDIR=$(readlink -f $(dirname $0))/..
cd $BASEDIR

# exit on errors
set -e

if ! hash nproc 2> /dev/null; then

	function nproc() {
		echo 8
	}

fi

function version_lt() {
	[ $(echo -e "$1\n$2" | sort -V | head -1) != "$1" ]
}

function array_contains_string() {
	name="$1[@]"
	array=("${!name}")

	for element in "${array[@]}"; do
		if [ "$element" = "$2" ]; then
			return $(true)
		fi
	done

	return $(false)
}

rc=0

echo -n "Checking file permissions..."

while read -r perm _res0 _res1 path; do
	if [ ! -f "$path" ]; then
		continue
	fi

	# Skip symlinks
	if [[ -L $path ]]; then
		continue
	fi
	fname=$(basename -- "$path")

	case ${fname##*.} in
		c | h | cpp | cc | cxx | hh | hpp | md | html | js | json | svg | Doxyfile | yml | LICENSE | README | conf | in | Makefile | mk | gitignore | go | txt)
			# These file types should never be executable
			if [ "$perm" -eq 100755 ]; then
				echo "ERROR: $path is marked executable but is a code file."
				rc=1
			fi
			;;
		*)
			shebang=$(head -n 1 $path | cut -c1-3)

			# git only tracks the execute bit, so will only ever return 755 or 644 as the permission.
			if [ "$perm" -eq 100755 ]; then
				# If the file has execute permission, it should start with a shebang.
				if [ "$shebang" != "#!/" ]; then
					echo "ERROR: $path is marked executable but does not start with a shebang."
					rc=1
				fi
			else
				# If the file doesnot have execute permissions, it should not start with a shebang.
				if [ "$shebang" = "#!/" ]; then
					echo "ERROR: $path is not marked executable but starts with a shebang."
					rc=1
				fi
			fi
			;;
	esac

done <<< "$(git grep -I --name-only --untracked -e . | git ls-files -s)"

if [ $rc -eq 0 ]; then
	echo " OK"
fi

if hash astyle; then
	echo -n "Checking coding style..."
	if [ "$(astyle -V)" \< "Artistic Style Version 3" ]; then
		echo -n " Your astyle version is too old so skipping coding style checks. Please update astyle to at least 3.0.1 version..."
	else
		rm -f astyle.log
		touch astyle.log
		# Exclude rte_vhost code imported from DPDK - we want to keep the original code
		#  as-is to enable ongoing work to synch with a generic upstream DPDK vhost library,
		#  rather than making diffs more complicated by a lot of changes to follow SPDK
		#  coding standards.
		git ls-files '*.[ch]' '*.cpp' '*.cc' '*.cxx' '*.hh' '*.hpp' \
			| grep -v rte_vhost | grep -v cpp_headers \
			| xargs -P$(nproc) -n10 astyle --options=.astylerc >> astyle.log
		if grep -q "^Formatted" astyle.log; then
			echo " errors detected"
			git diff --ignore-submodules=all
			sed -i -e 's/  / /g' astyle.log
			grep --color=auto "^Formatted.*" astyle.log
			echo "Incorrect code style detected in one or more files."
			echo "The files have been automatically formatted."
			echo "Remember to add the files to your commit."
			rc=1
		else
			echo " OK"
		fi
		rm -f astyle.log
	fi
else
	echo "You do not have astyle installed so your code style is not being checked!"
fi

GIT_VERSION=$(git --version | cut -d' ' -f3)

if version_lt "1.9.5" "${GIT_VERSION}"; then
	# git <1.9.5 doesn't support pathspec magic exclude
	echo " Your git version is too old to perform all tests. Please update git to at least 1.9.5 version..."
	exit 0
fi

echo -n "Checking comment style..."

git grep --line-number -e '\/[*][^ *-]' -- '*.[ch]' > comment.log || true
git grep --line-number -e '[^ ][*]\/' -- '*.[ch]' ':!lib/rte_vhost*/*' >> comment.log || true
git grep --line-number -e '^[*]' -- '*.[ch]' >> comment.log || true
git grep --line-number -e '\s\/\/' -- '*.[ch]' >> comment.log || true
git grep --line-number -e '^\/\/' -- '*.[ch]' >> comment.log || true

if [ -s comment.log ]; then
	echo " Incorrect comment formatting detected"
	cat comment.log
	rc=1
else
	echo " OK"
fi
rm -f comment.log

echo -n "Checking for spaces before tabs..."
git grep --line-number $' \t' -- './*' ':!*.patch' > whitespace.log || true
if [ -s whitespace.log ]; then
	echo " Spaces before tabs detected"
	cat whitespace.log
	rc=1
else
	echo " OK"
fi
rm -f whitespace.log

echo -n "Checking trailing whitespace in output strings..."

git grep --line-number -e ' \\n"' -- '*.[ch]' > whitespace.log || true

if [ -s whitespace.log ]; then
	echo " Incorrect trailing whitespace detected"
	cat whitespace.log
	rc=1
else
	echo " OK"
fi
rm -f whitespace.log

echo -n "Checking for use of forbidden library functions..."

git grep --line-number -w '\(atoi\|atol\|atoll\|strncpy\|strcpy\|strcat\|sprintf\|vsprintf\)' -- './*.c' ':!lib/rte_vhost*/**' > badfunc.log || true
if [ -s badfunc.log ]; then
	echo " Forbidden library functions detected"
	cat badfunc.log
	rc=1
else
	echo " OK"
fi
rm -f badfunc.log

echo -n "Checking for use of forbidden CUnit macros..."

git grep --line-number -w 'CU_ASSERT_FATAL' -- 'test/*' ':!test/spdk_cunit.h' > badcunit.log || true
if [ -s badcunit.log ]; then
	echo " Forbidden CU_ASSERT_FATAL usage detected - use SPDK_CU_ASSERT_FATAL instead"
	cat badcunit.log
	rc=1
else
	echo " OK"
fi
rm -f badcunit.log

echo -n "Checking blank lines at end of file..."

if ! git grep -I -l -e . -z './*' ':!*.patch' \
	| xargs -0 -P$(nproc) -n1 scripts/eofnl > eofnl.log; then
	echo " Incorrect end-of-file formatting detected"
	cat eofnl.log
	rc=1
else
	echo " OK"
fi
rm -f eofnl.log

echo -n "Checking for POSIX includes..."
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
if [ -s scripts/posix.log ]; then
	echo "POSIX includes detected. Please include spdk/stdinc.h instead."
	cat scripts/posix.log
	rc=1
else
	echo " OK"
fi
rm -f scripts/posix.log

echo -n "Checking for proper function naming conventions..."
# commit_to_compare = HEAD - 1.
commit_to_compare="$(git log --pretty=oneline --skip=1 -n 1 | awk '{print $1}')"
failed_naming_conventions=false
changed_c_libs=()
declared_symbols=()

# Build an array of all the modified C files.
mapfile -t changed_c_libs < <(git diff --name-only HEAD $commit_to_compare -- lib/**/*.c module/**/*.c)
# Matching groups are 1. qualifiers / return type. 2. function name 3. argument list / comments and stuff after that.
# Capture just the names of newly added (or modified) function definitions.
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')

for c_file in "${changed_c_libs[@]}"; do
	lib_map_file="mk/spdk_blank.map"
	defined_symbols=()
	removed_symbols=()
	exported_symbols=()
	if ls "$(dirname $c_file)"/*.map &> /dev/null; then
		lib_map_file="$(ls "$(dirname $c_file)"/*.map)"
	fi
	# Matching groups are 1. leading +sign. 2, function name 3. argument list / anything after that.
	# Capture just the names of newly added (or modified) functions that start with "spdk_"
	mapfile -t defined_symbols < <(git diff -U0 $commit_to_compare HEAD -- $c_file | sed -En 's/(^[+])(spdk[a-z,A-Z,0-9,_]*)(\(.*)/\2/p')
	# Capture the names of removed symbols to catch edge cases where we just move definitions around.
	mapfile -t removed_symbols < <(git diff -U0 $commit_to_compare HEAD -- $c_file | sed -En 's/(^[-])(spdk[a-z,A-Z,0-9,_]*)(\(.*)/\2/p')
	for symbol in "${removed_symbols[@]}"; do
		defined_symbols=("${defined_symbols[@]/$symbol/}")
	done
	# It's possible that we just modified a functions arguments so unfortunately we can't just look at changed lines in this function.
	# matching groups are 1. All leading whitespace 2. function name. Capture just the symbol name.
	mapfile -t exported_symbols < <(sed -En 's/(^[[:space:]]*)(spdk[a-z,A-Z,0-9,_]*);/\2/p' < $lib_map_file)
	for defined_symbol in "${defined_symbols[@]}"; do
		# 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.
		if [ "$defined_symbol" = '' ]; then
			continue
		fi
		not_exported=true
		not_declared=true
		if array_contains_string exported_symbols $defined_symbol; then
			not_exported=false
		fi

		if array_contains_string declared_symbols $defined_symbol; then
			not_declared=false
		fi

		if $not_exported || $not_declared; then
			if ! $failed_naming_conventions; then
				echo " found naming convention errors."
			fi
			echo "function $defined_symbol starts with spdk_ which is reserved for public API functions."
			echo "Please add this function to its corresponding map file and a public header or remove the spdk_ prefix."
			failed_naming_conventions=true
			rc=1
		fi
	done
done

if ! $failed_naming_conventions; then
	echo " OK"
fi

echo -n "Checking #include style..."
git grep -I -i --line-number "#include <spdk/" -- '*.[ch]' > scripts/includes.log || true
if [ -s scripts/includes.log ]; then
	echo "Incorrect #include syntax. #includes of spdk/ files should use quotes."
	cat scripts/includes.log
	rc=1
else
	echo " OK"
fi
rm -f scripts/includes.log

if hash pycodestyle 2> /dev/null; then
	PEP8=pycodestyle
elif hash pep8 2> /dev/null; then
	PEP8=pep8
fi

if [ -n "${PEP8}" ]; then
	echo -n "Checking Python style..."

	PEP8_ARGS+=" --max-line-length=140"

	error=0
	git ls-files '*.py' | xargs -P$(nproc) -n1 $PEP8 $PEP8_ARGS > pep8.log || error=1
	if [ $error -ne 0 ]; then
		echo " Python formatting errors detected"
		cat pep8.log
		rc=1
	else
		echo " OK"
	fi
	rm -f pep8.log
else
	echo "You do not have pycodestyle or pep8 installed so your Python style is not being checked!"
fi

# find compatible shfmt binary
shfmt_bins=$(compgen -c | grep '^shfmt' || true)
for bin in $shfmt_bins; do
	if version_lt "$("$bin" --version)" "3.1.0"; then
		shfmt=$bin
		break
	fi
done

if [ -n "$shfmt" ]; then
	shfmt_cmdline=() silly_plural=()

	silly_plural[1]="s"

	commits=() sh_files=() sh_files_repo=() sh_files_staged=()

	mapfile -t sh_files_repo < <(git ls-files '*.sh')
	# Fetch .sh files only from the commits that are targeted for merge
	while read -r _ commit; do
		commits+=("$commit")
	done < <(git cherry -v origin/master)

	mapfile -t sh_files < <(git diff --name-only HEAD origin/master "${sh_files_repo[@]}")
	# In case of a call from a pre-commit git hook
	mapfile -t sh_files_staged < <(
		IFS="|"
		git diff --cached --name-only "${sh_files_repo[@]}" | grep -v "${sh_files[*]}"
	)

	if ((${#sh_files[@]})); then
		printf 'Checking .sh formatting style...'

		if ((${#sh_files_staged[@]})); then
			sh_files+=("${sh_files_staged[@]}")
		fi

		shfmt_cmdline+=(-i 0)     # indent_style = tab|indent_size = 0
		shfmt_cmdline+=(-bn)      # binary_next_line = true
		shfmt_cmdline+=(-ci)      # switch_case_indent = true
		shfmt_cmdline+=(-ln bash) # shell_variant = bash (default)
		shfmt_cmdline+=(-d)       # diffOut - print diff of the changes and exit with != 0
		shfmt_cmdline+=(-sr)      # redirect operators will be followed by a space

		diff=${output_dir:-$PWD}/$shfmt.patch

		# Explicitly tell shfmt to not look for .editorconfig. .editorconfig is also not looked up
		# in case any formatting arguments has been passed on its cmdline.
		if ! SHFMT_NO_EDITORCONFIG=true "$shfmt" "${shfmt_cmdline[@]}" "${sh_files[@]}" > "$diff"; then
			# In case shfmt detects an actual syntax error it will write out a proper message on
			# its stderr, hence the diff file should remain empty.
			if [[ -s $diff ]]; then
				diff_out=$(< "$diff")
			fi

			cat <<- ERROR_SHFMT

				* Errors in style formatting have been detected.
				${diff_out:+* Please, review the generated patch at $diff

				# _START_OF_THE_DIFF

				${diff_out:-ERROR}

				# _END_OF_THE_DIFF
				}

			ERROR_SHFMT
			rc=1
		else
			rm -f "$diff"
			printf ' OK\n'
		fi
	fi
else
	echo "shfmt not detected, Bash style formatting check is skipped"
fi

if hash shellcheck 2> /dev/null; then
	echo -n "Checking Bash style..."

	shellcheck_v=$(shellcheck --version | grep -P "version: [0-9\.]+" | cut -d " " -f2)

	# SHCK_EXCLUDE contains a list of all of the spellcheck errors found in SPDK scripts
	# currently. New errors should only be added to this list if the cost of fixing them
	# is deemed too high. For more information about the errors, go to:
	# https://github.com/koalaman/shellcheck/wiki/Checks
	# Error descriptions can also be found at: https://github.com/koalaman/shellcheck/wiki
	# SPDK fails some error checks which have been deprecated in later versions of shellcheck.
	# We will not try to fix these error checks, but instead just leave the error types here
	# so that we can still run with older versions of shellcheck.
	SHCK_EXCLUDE="SC1117"
	# SPDK has decided to not fix violations of these errors.
	# We are aware about below exclude list and we want this errors to be excluded.
	# SC1083: This {/} is literal. Check expression (missing ;/\n?) or quote it.
	# SC1090: Can't follow non-constant source. Use a directive to specify location.
	# SC1091: Not following: (error message here)
	# SC2001: See if you can use ${variable//search/replace} instead.
	# SC2010: Don't use ls | grep. Use a glob or a for loop with a condition to allow non-alphanumeric filenames.
	# SC2015: Note that A && B || C is not if-then-else. C may run when A is true.
	# SC2016: Expressions don't expand in single quotes, use double quotes for that.
	# SC2034: foo appears unused. Verify it or export it.
	# SC2046: Quote this to prevent word splitting.
	# SC2086: Double quote to prevent globbing and word splitting.
	# SC2119: Use foo "$@" if function's $1 should mean script's $1.
	# SC2120: foo references arguments, but none are ever passed.
	# SC2148: Add shebang to the top of your script.
	# SC2153: Possible Misspelling: MYVARIABLE may not be assigned, but MY_VARIABLE is.
	# SC2154: var is referenced but not assigned.
	# SC2164: Use cd ... || exit in case cd fails.
	# SC2174: When used with -p, -m only applies to the deepest directory.
	# SC2206: Quote to prevent word splitting/globbing,
	#         or split robustly with mapfile or read -a.
	# SC2207: Prefer mapfile or read -a to split command output (or quote to avoid splitting).
	# SC2223: This default assignment may cause DoS due to globbing. Quote it.
	SHCK_EXCLUDE="$SHCK_EXCLUDE,SC1083,SC1090,SC1091,SC2010,SC2015,SC2016,SC2034,SC2046,SC2086,\
SC2119,SC2120,SC2148,SC2153,SC2154,SC2164,SC2174,SC2001,SC2206,SC2207,SC2223"

	SHCK_FORMAT="diff"
	SHCK_APPLY=true
	if [ "$shellcheck_v" \< "0.7.0" ]; then
		SHCK_FORMAT="tty"
		SHCK_APPLY=false
	fi
	SHCH_ARGS=" -x -e $SHCK_EXCLUDE -f $SHCK_FORMAT"

	error=0
	git ls-files '*.sh' | xargs -P$(nproc) -n1 shellcheck $SHCH_ARGS &> shellcheck.log || error=1
	if [ $error -ne 0 ]; then
		echo " Bash formatting errors detected!"

		# Some errors are not auto-fixable. Fall back to tty output.
		if grep -q "Use another format to see them." shellcheck.log; then
			SHCK_FORMAT="tty"
			SHCK_APPLY=false
			SHCH_ARGS=" -e $SHCK_EXCLUDE -f $SHCK_FORMAT"
			git ls-files '*.sh' | xargs -P$(nproc) -n1 shellcheck $SHCH_ARGS > shellcheck.log || error=1
		fi

		cat shellcheck.log
		if $SHCK_APPLY; then
			git apply shellcheck.log
			echo "Bash errors were automatically corrected."
			echo "Please remember to add the changes to your commit."
		fi
		rc=1
	else
		echo " OK"
	fi
	rm -f shellcheck.log
else
	echo "You do not have shellcheck installed so your Bash style is not being checked!"
fi

# Check if any of the public interfaces were modified by this patch.
# Warn the user to consider updating the changelog any changes
# are detected.
echo -n "Checking whether CHANGELOG.md should be updated..."
staged=$(git diff --name-only --cached .)
working=$(git status -s --porcelain --ignore-submodules | grep -iv "??" | awk '{print $2}')
files="$staged $working"
if [[ "$files" = " " ]]; then
	files=$(git diff-tree --no-commit-id --name-only -r HEAD)
fi

has_changelog=0
for f in $files; do
	if [[ $f == CHANGELOG.md ]]; then
		# The user has a changelog entry, so exit.
		has_changelog=1
		break
	fi
done

needs_changelog=0
if [ $has_changelog -eq 0 ]; then
	for f in $files; do
		if [[ $f == include/spdk/* ]] || [[ $f == scripts/rpc.py ]] || [[ $f == etc/* ]]; then
			echo ""
			echo -n "$f was modified. Consider updating CHANGELOG.md."
			needs_changelog=1
		fi
	done
fi

if [ $needs_changelog -eq 0 ]; then
	echo " OK"
else
	echo ""
fi

exit $rc