From f607739cba154a0fa23d5e413c21373af8d94b2c Mon Sep 17 00:00:00 2001 From: Sandrine Bailleux Date: Thu, 2 Jun 2016 11:19:59 +0100 Subject: [PATCH] Move checkpatch options in a configuration file At the moment, the top Makefile specifies the options to pass to the checkpatch script in order to check the coding style. The checkpatch script also supports reading its options from a configuration file rather than from the command line. This patch makes use of this feature and moves the checkpatch options out of the Makefile. This simplifies the Makefile and makes things clearer. This patch also adds some more checkpatch options: --showfile --ignore FILE_PATH_CHANGES --ignore AVOID_EXTERNS --ignore NEW_TYPEDEFS --ignore VOLATILE The rationale behind each of these options has been documented in the configuration file. Change-Id: I423e1abe5670c0f57046cbf705f89a8463898676 --- .checkpatch.conf | 87 ++++++++++++++++++++++++++++++++++++++++++++++++ Makefile | 8 ++--- 2 files changed, 89 insertions(+), 6 deletions(-) create mode 100644 .checkpatch.conf diff --git a/.checkpatch.conf b/.checkpatch.conf new file mode 100644 index 000000000..c8a6084b5 --- /dev/null +++ b/.checkpatch.conf @@ -0,0 +1,87 @@ +# +# Copyright (c) 2016, ARM Limited and Contributors. All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# +# Redistributions of source code must retain the above copyright notice, this +# list of conditions and the following disclaimer. +# +# Redistributions in binary form must reproduce the above copyright notice, +# this list of conditions and the following disclaimer in the documentation +# and/or other materials provided with the distribution. +# +# Neither the name of ARM nor the names of its contributors may be used +# to endorse or promote products derived from this software without specific +# prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +# ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE +# LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR +# CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF +# SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN +# CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) +# ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +# POSSIBILITY OF SUCH DAMAGE. +# + +# +# Configure how the Linux checkpatch script should be invoked in the context of +# the Trusted Firmware source tree. +# + +# This is not Linux so don't expect a Linux tree! +--no-tree + +# 'Signed-off-by' lines in commit messages are not mandated for TF. +--no-signoff + +# This clarifes the lines indications in the report. +# +# E.g.: +# Without this option, we have the following output: +# #333: FILE: drivers/arm/gic/arm_gic.c:160: +# So we have 2 lines indications (333 and 160), which is confusing. +# We only care about the position in the source file. +# +# With this option, it becomes: +# drivers/arm/gic/arm_gic.c:160: +--showfile + +# +# Ignore the following message types, as they don't necessarily make sense in +# the context of the Trusted Firmware. +# + +# COMPLEX_MACRO generates false positives. +--ignore COMPLEX_MACRO + +# Commit messages might contain a Gerrit Change-Id. +--ignore GERRIT_CHANGE_ID + +# Do not check the format of commit messages, as Github's merge commits do not +# observe it. +--ignore GIT_COMMIT_ID + +# FILE_PATH_CHANGES reports this kind of message: +# "added, moved or deleted file(s), does MAINTAINERS need updating?" +# We do not use this MAINTAINERS file process in TF. +--ignore FILE_PATH_CHANGES + +# AVOID_EXTERNS reports this kind of messages: +# "externs should be avoided in .c files" +# We don't follow this convention in TF. +--ignore AVOID_EXTERNS + +# NEW_TYPEDEFS reports this kind of messages: +# "do not add new typedefs" +# We allow adding new typedefs in TF. +--ignore NEW_TYPEDEFS + +# VOLATILE reports this kind of messages: +# "Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt" +# We allow the usage of the volatile keyword in TF. +--ignore VOLATILE diff --git a/Makefile b/Makefile index 9d183d218..74bbe4265 100644 --- a/Makefile +++ b/Makefile @@ -107,11 +107,7 @@ PL011_GENERIC_UART := 0 # Checkpatch script options ################################################################################ -CHECK_IGNORE := --ignore COMPLEX_MACRO \ - --ignore GERRIT_CHANGE_ID \ - --ignore GIT_COMMIT_ID -CHECKPATCH_ARGS := --no-tree --no-signoff ${CHECK_IGNORE} -CHECKCODE_ARGS := --no-patch --no-tree --no-signoff ${CHECK_IGNORE} +CHECKCODE_ARGS := --no-patch # Do not check the coding style on imported library files or documentation files INC_LIB_DIRS_TO_CHECK := $(sort $(filter-out \ include/lib/libfdt \ @@ -571,7 +567,7 @@ checkcodebase: locate-checkpatch checkpatch: locate-checkpatch @echo " CHECKING STYLE" - ${Q}git log -p ${BASE_COMMIT}..HEAD -- ${CHECK_PATHS} | ${CHECKPATCH} ${CHECKPATCH_ARGS} - || true + ${Q}git log -p ${BASE_COMMIT}..HEAD -- ${CHECK_PATHS} | ${CHECKPATCH} - || true certtool: ${CRTTOOL}