From patchwork Mon Oct 3 10:36:38 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1685502 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=2605:bc80:3010::136; helo=smtp3.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: legolas.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=b+LOfcaR; dkim-atps=neutral Received: from smtp3.osuosl.org (smtp3.osuosl.org [IPv6:2605:bc80:3010::136]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4Mgy262wlnz23jL for ; Mon, 3 Oct 2022 21:37:02 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id CC78B60BFD; Mon, 3 Oct 2022 10:36:59 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org CC78B60BFD Authentication-Results: smtp3.osuosl.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=b+LOfcaR X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Sr3njc1kp2Kv; Mon, 3 Oct 2022 10:36:58 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp3.osuosl.org (Postfix) with ESMTPS id 080ED60B9E; Mon, 3 Oct 2022 10:36:57 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 080ED60B9E Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id D26CAC0032; Mon, 3 Oct 2022 10:36:56 +0000 (UTC) X-Original-To: ovs-dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp3.osuosl.org (smtp3.osuosl.org [IPv6:2605:bc80:3010::136]) by lists.linuxfoundation.org (Postfix) with ESMTP id 869D4C0032 for ; Mon, 3 Oct 2022 10:36:55 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 931ED607C1 for ; Mon, 3 Oct 2022 10:36:48 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 931ED607C1 X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id fM15lLIBo9B6 for ; Mon, 3 Oct 2022 10:36:46 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 7611C60BF3 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by smtp3.osuosl.org (Postfix) with ESMTPS id 7611C60BF3 for ; Mon, 3 Oct 2022 10:36:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1664793405; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=/m6uERs+B7m4cnAvTy66ZfuVOl1caEBFjI1W9dqqv1g=; b=b+LOfcaRel5+0jbrjchUPeYn57n+GD71vMyiZlCpR6KngoXn+QoHNQY9s9Lunkyce7fpsL N4bIkxBOIdaNrORBzRQN/jRmj85IFKHQZ3d2SYzDK8/RQ4wPAXQazFDxbtK6kgUhEFRVAv dJYhGufsVuR5HSRt0z0BeTn6FXDLJTM= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-5-QhCq9VBTOYqZ88Y1S4_JzA-1; Mon, 03 Oct 2022 06:36:42 -0400 X-MC-Unique: QhCq9VBTOYqZ88Y1S4_JzA-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.rdu2.redhat.com [10.11.54.2]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 0E33D29AB44B; Mon, 3 Oct 2022 10:36:42 +0000 (UTC) Received: from dceara.remote.csb (unknown [10.39.194.29]) by smtp.corp.redhat.com (Postfix) with ESMTP id 0E16E40C6EC2; Mon, 3 Oct 2022 10:36:40 +0000 (UTC) From: Dumitru Ceara To: ovs-dev@openvswitch.org Date: Mon, 3 Oct 2022 12:36:38 +0200 Message-Id: <166479339632.310761.2218770635645253107.stgit@dceara.remote.csb> In-Reply-To: <166479337525.310761.1104428475550955568.stgit@dceara.remote.csb> References: <166479337525.310761.1104428475550955568.stgit@dceara.remote.csb> User-Agent: StGit/0.23 MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.2 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Cc: i.maximets@ovn.org Subject: [ovs-dev] [PATCH ovn 2/2] checkpatch.py: Port checkpatch related changes from the OVS repo. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" This ports and squashes the following OVS commits: commit 950aee1f7e1113a030b5e62ed036608480118b9d Author: Ilya Maximets Date: Wed Sep 14 13:06:38 2022 +0200 checkpatch: Add check for egrep/fgrep. GNU grep 3.8 started complaining about use of obsolete egrep/fgrep: egrep: warning: egrep is obsolescent; using grep -E This breaks tests on such systems. All the instances was cleaned up from the testsuite, but the checkpatch check is needed to catch issues in new patches. Acked-by: Aaron Conole Signed-off-by: Ilya Maximets commit 91b41af0d9b33ea9f3f739cae4d0e20a30330cae Author: Ilya Maximets Date: Tue Jul 26 15:29:55 2022 +0200 checkpatch: Add check for a Fixes tag. A new check for common mistakes while formatting a 'Fixes:' tag. Acked-by: Sunil Pai G Signed-off-by: Ilya Maximets commit 9f4f2bb7dc1df888878902e8b11fef886af928a2 Author: Frode Nordahl Date: Thu Jul 14 17:55:39 2022 +0200 checkpatch: Ignore line length and leading whitespace for debian/*. Signed-off-by: Frode Nordahl Signed-off-by: Ilya Maximets commit 071b802c61a3bf0b9304764b4cbc10dfad972105 Author: Peng He Date: Thu May 26 03:13:18 2022 +0000 checkpatch.py: Add checks for easy-to-misuse APIs. Signed-off-by: Peng He Acked-by: Eelco Chaudron Acked-by: Aaron Conole Signed-off-by: Ilya Maximets commit 0d1ffb77560fdbb96bded347fad59abd5798bb29 Author: Mike Pattrick Date: Tue Nov 30 11:20:53 2021 -0500 checkpatch: Detect "trojan source" attack. Recently there has been a lot of press about the "trojan source" attack, where Unicode characters are used to obfuscate the true functionality of code. This attack didn't effect OVS, but adding the check here will help guard against it sneaking in later. Signed-off-by: Mike Pattrick Acked-by: Gaetan Rivet Signed-off-by: Ilya Maximets commit d652fc6a5a6c78b0daeb389f28933570bb4ecb56 Author: Mike Pattrick Date: Tue Nov 30 11:02:07 2021 -0500 checkpatch: Correct line count in error messages. As part of some previous checkpatch work, we discovered that checkpatch isn't always reporting correct line numbers. As it turns out, Python's splitlines function considers several characters to be new lines which common text editors do not typically consider to be new lines. For example, form feed characters, which this code base uses to cluster functionality. Signed-off-by: Mike Pattrick Acked-by: Paolo Valerio Signed-off-by: Ilya Maximets commit c5d384f77bed0e3f540987314072166bfcbff1a2 Author: Timothy Redaelli Date: Fri Sep 10 11:08:14 2021 +0200 checkpatch: Check if some tags are wrongly written. Currently, there are some patches with the tags wrongly written (with space instead of dash ) and this may prevent some automatic system or CI to detect them correctly. This commit adds a check in checkpatch to be sure the tag is written correctly with dash and not with space. The tags supported by the commit are: Acked-by, Reported-at, Reported-by, Requested-by, Reviewed-by, Submitted-at and Suggested-by. It's not necessary to add "Signed-off-by" since it's already checked in checkpatch. Signed-off-by: Timothy Redaelli Signed-off-by: Ilya Maximets commit b6c5f30cfa9994a1069bc6bef28a270bbb61df6c Author: Aaron Conole Date: Fri Jun 25 13:18:07 2021 -0400 checkpatch: Ignore macro definitions of FOR_EACH. When defining a FOR_EACH macro, checkpatch freaks out and generates a control block whitespace error. Create an exception so that it doesn't generate errors for this case. Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2020-August/373509.html Reported-by: Toshiaki Makita Signed-off-by: Aaron Conole Signed-off-by: Ilya Maximets commit dc497e36fc06a72e9f66674ab947d5b3d98b7f5f Author: Ilya Maximets Date: Wed Nov 18 22:18:58 2020 +0100 checkpatch: Add check for a whitespace after cast. Coding style says: "Put a space between the ``()`` used in a cast and the expression whose type is cast: ``(void *) 0``.". This style rule is frequently overlooked. Let's check for it. Signed-off-by: Ilya Maximets Acked-by: Ian Stokes commit 68a95c9ca7cea83f91c7fd152608e8ee6621013d Author: Roi Dayan Date: Tue Jul 14 10:24:41 2020 +0300 checkpatch: Add argument to skip gerrit change id check. This arg can be used internally by groups using gerrit for code reviews. Acked-by: Flavio Leitner Signed-off-by: Roi Dayan Signed-off-by: Ilya Maximets commit 9802fafa962baff8f52aeea1f1139a68c027d1d2 Author: Ilya Maximets Date: Sat Dec 7 18:14:00 2019 +0100 checkpatch: Check spelling in commit messages. This seems useful as I'm usually making a lot of typing mistakes. Also, few commonly used words added to the extended dictionary. Signed-off-by: Ilya Maximets Acked-by: Aaron Conole Acked-by: William Tu commit 37c6dfab1002e212d2c8717bdc55f09d3668ef16 Author: Ilya Maximets Date: Sat Dec 7 18:10:24 2019 +0100 checkpatch: Skip words containing numbers. Words like 'br0' are common and usually references some code or database objects that should not be targets for spell checking. So, it's better to skip all the words that has digits inside instead of ones that starts with numbers. Signed-off-by: Ilya Maximets Acked-by: Aaron Conole Acked-by: William Tu commit 98a411b32e3f94bc711a01bb5ad18488eee6eed7 Author: Ilya Maximets Date: Sat Dec 7 18:02:01 2019 +0100 checkpatch: Allow common abbreviations for spell checking. Abbreviations of Latin expressions like 'i.e.' or 'e.g.' are common and known by the dictionary. However, our spell checker is not able to recognize them because it strips dots out of them. To avoid this issue we could pass non-stripped version of the word to the dictionary checker too. Signed-off-by: Ilya Maximets Acked-by: Aaron Conole Acked-by: William Tu commit 334eec6a8ece99a0145165352ad38636f697c548 Author: William Tu Date: Thu Sep 12 11:11:17 2019 -0700 checkpatch: Ignore utilities/bugtool. Reviewed-by: Greg Rose Signed-off-by: William Tu Signed-off-by: Ben Pfaff Signed-off-by: Dumitru Ceara --- tests/checkpatch.at | 224 ++++++++++++++++++++++++++++++++++++++++++++++- utilities/checkpatch.py | 171 ++++++++++++++++++++++++++++++------ 2 files changed, 365 insertions(+), 30 deletions(-) diff --git a/tests/checkpatch.at b/tests/checkpatch.at index 0c724d3d61..db564069f4 100755 --- a/tests/checkpatch.at +++ b/tests/checkpatch.at @@ -167,10 +167,10 @@ m4_define([COMMON_PATCH_HEADER], [dnl Signed-off-by: A --- - diff --git a/A.c b/A.c + diff --git a/$([[ ! -z "$1" ]] && echo "$1" || echo "A.c") b/$([[ ! -z "$1" ]] && echo "$1" || echo "A.c") index 0000000..1111111 100644 - --- a/A.c - +++ b/A.c + --- a/$([[ ! -z "$1" ]] && echo "$1" || echo "A.c") + +++ b/$([[ ! -z "$1" ]] && echo "$1" || echo "A.c") @@ -1,1 +1,1 @@]) @@ -287,6 +287,11 @@ try_checkpatch \ + for (init; condition; increment) { \\ " +try_checkpatch \ + "COMMON_PATCH_HEADER + +#define SOME_FOR_EACH(a, b, c) /* Foo. */ + " + AT_CLEANUP @@ -341,3 +346,216 @@ try_checkpatch \ " AT_CLEANUP + +AT_SETUP([checkpatch - check misuse APIs]) +try_checkpatch \ + "COMMON_PATCH_HEADER([a.c]) + + ovsrcu_barrier(); + " \ + "WARNING: Are you sure you need to use ovsrcu_barrier(), "\ +"in most cases ovsrcu_synchronize() will be fine? + #8 FILE: a.c:1: + ovsrcu_barrier(); +" + +try_checkpatch \ + "COMMON_PATCH_HEADER([lib/ovs-rcu.c]) + + ovsrcu_barrier(); + " + +AT_CLEANUP + + +AT_SETUP([checkpatch - check egrep / fgrep]) +try_checkpatch \ + "COMMON_PATCH_HEADER([tests/something.at]) + +C_H_E_C_K([[ovs-vsctl show | grep -E 'my-port.*[[0-9]]$' | grep -F 'port']]) + " + +try_checkpatch \ + "COMMON_PATCH_HEADER([tests/something.at]) + +C_H_E_C_K([[ovs-vsctl show | egrep 'my-port.*[[0-9]]$']]) + " \ + "ERROR: grep -E/-F should be used instead of egrep/fgrep + #8 FILE: tests/something.at:1: + C_H_E_C_K([[ovs-vsctl show | egrep 'my-port.*[[0-9]]$']]) +" + +try_checkpatch \ + "COMMON_PATCH_HEADER([tests/something.at]) + +C_H_E_C_K([[ovs-vsctl show | fgrep 'my-port.*[[0-9]]$']]) + " \ + "ERROR: grep -E/-F should be used instead of egrep/fgrep + #8 FILE: tests/something.at:1: + C_H_E_C_K([[ovs-vsctl show | fgrep 'my-port.*[[0-9]]$']]) +" +AT_CLEANUP + + +AT_SETUP([checkpatch - whitespace around cast]) +try_checkpatch \ + "COMMON_PATCH_HEADER + + (int) a; + " + +try_checkpatch \ + "COMMON_PATCH_HEADER + + (int)a; + " \ + "ERROR: Inappropriate spacing around cast + #8 FILE: A.c:1: + (int)a; +" + +AT_CLEANUP + +AT_SETUP([checkpatch - malformed tags]) +try_checkpatch \ + " Author: A + + Acked by: foo... + Signed-off-by: A" \ + "ERROR: Acked-by tag is malformed. + 1: Acked by: foo... +" +try_checkpatch \ + " Author: A + + Reported at: foo... + Signed-off-by: A" \ + "ERROR: Reported-at tag is malformed. + 1: Reported at: foo... +" +try_checkpatch \ + " Author: A + + Reported by: foo... + Signed-off-by: A" \ + "ERROR: Reported-by tag is malformed. + 1: Reported by: foo... +" +try_checkpatch \ + " Author: A + + Requested by: foo... + Signed-off-by: A" \ + "ERROR: Requested-by tag is malformed. + 1: Requested by: foo... +" +try_checkpatch \ + " Author: A + + Reviewed by: foo... + Signed-off-by: A" \ + "ERROR: Reviewed-by tag is malformed. + 1: Reviewed by: foo... +" +try_checkpatch \ + " Author: A + + Submitted at: foo... + Signed-off-by: A" \ + "ERROR: Submitted-at tag is malformed. + 1: Submitted at: foo... +" +try_checkpatch \ + " Author: A + + Suggested by: foo... + Signed-off-by: A" \ + "ERROR: Suggested-by tag is malformed. + 1: Suggested by: foo... +" + +AT_CLEANUP + +AT_SETUP([checkpatch - Unicode code]) +try_checkpatch \ + "COMMON_PATCH_HEADER + + if (snowman == ☃️) { /* Emoji + + void НelloWorld() { /* Homoglyph + + ة /* ;C++ /* BiDi + " \ + "ERROR: Inappropriate non-ascii characters detected. + #8 FILE: A.c:1: + if (snowman == ☃️) { /* Emoji + +ERROR: Inappropriate non-ascii characters detected. + #9 FILE: A.c:2: + void НelloWorld() { /* Homoglyph + +ERROR: Inappropriate non-ascii characters detected. + #10 FILE: A.c:3: + ة /* ;C++ /* BiDi +" + +AT_CLEANUP + + +m4_define([FIXES_TAG_ERROR], [dnl +ERROR: \"Fixes\" tag is malformed. +Use the following format: + git log -1 --pretty=format:\"Fixes: %h (\\\"%s\\\")\" --abbrev=12 COMMIT_REF +]) + +AT_SETUP([checkpatch - Fixes tag]) + +try_checkpatch \ + "Author: A + Commit: A + + Fixes: 123456789abc (\"commit name\") + Signed-off-by: A" + +try_checkpatch \ + "Author: A + Commit: A + + fixes: 123456789abc (\"commit name\") + Signed-off-by: A" \ + "FIXES_TAG_ERROR +1: fixes: 123456789abc (\"commit name\") +" + +try_checkpatch \ + "Author: A + Commit: A + + Fixes: 12345678 (\"commit name\") + Signed-off-by: A" \ + "FIXES_TAG_ERROR +1: Fixes: 12345678 (\"commit name\") +" + +try_checkpatch \ + "Author: A + Commit: A + + Fixes: 1234567890abcdef1234567890abcdef12345678 (\"commit name\") + Signed-off-by: A" \ + "FIXES_TAG_ERROR +1: Fixes: 1234567890abcdef1234567890abcdef12345678 (\"commit name\") +" + +try_checkpatch \ + "Author: A + Commit: A + + Fixes: 123456789abc \"commit name\" + Signed-off-by: A" \ + "FIXES_TAG_ERROR +1: Fixes: 123456789abc \"commit name\" +" + +try_checkpatch \ + "Author: A + Commit: A + + Fixes: 123456789abc (\"some very long commit name that doesn\'t fit into + a single line, but should not be wrapped.\") + Signed-off-by: A" \ + "FIXES_TAG_ERROR +1: Fixes: 123456789abc (\"some very long commit name that doesn\'t fit into +" + +AT_CLEANUP diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py index ffb873111e..da3224bbca 100755 --- a/utilities/checkpatch.py +++ b/utilities/checkpatch.py @@ -31,7 +31,7 @@ print_file_name = None checking_file = False total_line = 0 colors = False -spellcheck_comments = False +spellcheck = False quiet = False spell_check_dict = None MAX_LINE_LEN = 79 @@ -84,7 +84,12 @@ def open_spell_check_dict(): 'cacheline', 'xlate', 'skiplist', 'idl', 'comparator', 'natting', 'alg', 'pasv', 'epasv', 'wildcard', 'nated', 'amd64', 'x86_64', - 'recirculation'] + 'recirculation', 'linux', 'afxdp', 'promisc', 'goto', + 'misconfigured', 'misconfiguration', 'checkpatch', + 'debian', 'travis', 'cirrus', 'appveyor', 'faq', + 'erspan', 'const', 'hotplug', 'addresssanitizer', + 'ovsdb', 'dpif', 'veth', 'rhel', 'jsonrpc', 'json', + 'syscall', 'lacp', 'ipf', 'skb', 'valgrind'] global spell_check_dict spell_check_dict = enchant.Dict("en_US") @@ -153,6 +158,8 @@ __regex_trailing_whitespace = re.compile(r'[^\S]+$') __regex_single_line_feed = re.compile(r'^\f$') __regex_for_if_missing_whitespace = re.compile(r' +(%s)[\(]' % __parenthesized_constructs) +__regex_hash_define_for_each = re.compile( + r'#define [_A-Z]+FOR_*EACH[_A-Z0-9]*\(') __regex_for_if_too_much_whitespace = re.compile(r' +(%s) +[\(]' % __parenthesized_constructs) __regex_for_if_parens_whitespace = \ @@ -162,6 +169,7 @@ __regex_is_for_if_single_line_bracket = \ __regex_ends_with_bracket = \ re.compile(r'[^\s]\) {(\s+/\*[\s\Sa-zA-Z0-9\.,\?\*/+-]*)?$') __regex_ptr_declaration_missing_whitespace = re.compile(r'[a-zA-Z0-9]\*[^*]') +__regex_cast_missing_whitespace = re.compile(r'\)[a-zA-Z0-9]') __regex_is_comment_line = re.compile(r'^\s*(/\*|\*\s)') __regex_has_comment = re.compile(r'.*(/\*|\*\s)') __regex_has_c99_comment = re.compile(r'.*//.*$') @@ -174,9 +182,12 @@ __regex_added_doc_rst = re.compile( __regex_empty_return = re.compile(r'\s*return;') __regex_if_macros = re.compile(r'^ +(%s) \([\S]([\s\S]+[\S])*\) { +\\' % __parenthesized_constructs) +__regex_nonascii_characters = re.compile("[^\u0000-\u007f]") +__regex_efgrep = re.compile(r'.*[ef]grep.*$') skip_leading_whitespace_check = False skip_trailing_whitespace_check = False +skip_gerrit_change_id_check = False skip_block_whitespace_check = False skip_signoff_check = False @@ -185,13 +196,12 @@ skip_signoff_check = False # # Python isn't checked as flake8 performs these checks during build. line_length_blacklist = re.compile( - r'\.(am|at|etc|in|m4|mk|patch|py|dl)$|debian/rules') + r'\.(am|at|etc|in|m4|mk|patch|py)$|^debian/.*$') # Don't enforce a requirement that leading whitespace be all spaces on # files that include these characters in their name, since these kinds # of files need lines with leading tabs. -leading_whitespace_blacklist = re.compile( - r'\.(mk|am|at)$|debian/rules|\.gitmodules$') +leading_whitespace_blacklist = re.compile(r'\.(mk|am|at)$|^debian/.*$') def is_subtracted_line(line): @@ -240,9 +250,11 @@ def if_and_for_whitespace_checks(line): """ if skip_block_whitespace_check: return True - if (__regex_for_if_missing_whitespace.search(line) is not None or - __regex_for_if_too_much_whitespace.search(line) is not None or - __regex_for_if_parens_whitespace.search(line)): + if (__regex_for_if_missing_whitespace.search(line) is not None and + __regex_hash_define_for_each.search(line) is None): + return False + if (__regex_for_if_too_much_whitespace.search(line) is not None or + __regex_for_if_parens_whitespace.search(line)): return False return True @@ -286,6 +298,17 @@ def pointer_whitespace_check(line): return __regex_ptr_declaration_missing_whitespace.search(line) is not None +def nonascii_character_check(line): + """Return TRUE if inappropriate Unicode characters are detected """ + return __regex_nonascii_characters.search(line) is not None + + +def cast_whitespace_check(line): + """Return TRUE if there is no space between the '()' used in a cast and + the expression whose type is cast, i.e.: '(void *)foo'""" + return __regex_cast_missing_whitespace.search(line) is not None + + def line_length_check(line): """Return TRUE if the line length is too long""" if len(line) > MAX_LINE_LEN: @@ -321,6 +344,11 @@ def has_xxx_mark(line): return __regex_has_xxx_mark.match(line) is not None +def has_efgrep(line): + """Returns TRUE if the current line contains 'egrep' or 'fgrep'.""" + return __regex_efgrep.match(line) is not None + + def filter_comments(current_line, keep=False): """remove all of the c-style comments in a line""" STATE_NORMAL = 0 @@ -378,15 +406,19 @@ def filter_comments(current_line, keep=False): return sanitized_line -def check_comment_spelling(line): - if not spell_check_dict or not spellcheck_comments: +def check_spelling(line, comment): + if not spell_check_dict or not spellcheck: return False - comment_words = filter_comments(line, True).replace(':', ' ').split(' ') - for word in comment_words: + words = filter_comments(line, True) if comment else line + words = words.replace(':', ' ').split(' ') + + for word in words: skip = False strword = re.subn(r'\W+', '', word)[0].replace(',', '') - if len(strword) and not spell_check_dict.check(strword.lower()): + if (len(strword) + and not spell_check_dict.check(strword.lower()) + and not spell_check_dict.check(word.lower())): if any([check_char in word for check_char in ['=', '(', '-', '_', '/', '\'']]): skip = True @@ -400,8 +432,8 @@ def check_comment_spelling(line): strword[1:].islower()): skip = True - # skip words that start with numbers - if strword.startswith(tuple('0123456789')): + # skip words containing numbers + if any(check_char.isdigit() for check_char in strword): skip = True if not skip: @@ -547,6 +579,17 @@ checks = [ 'print': lambda: print_error("Inappropriate spacing in pointer declaration")}, + {'regex': r'(\.c|\.h)(\.in)?$', 'match_name': None, + 'check': lambda x: nonascii_character_check(x), + 'print': + lambda: print_error("Inappropriate non-ascii characters detected.")}, + + {'regex': r'(\.c|\.h)(\.in)?$', 'match_name': None, + 'prereq': lambda x: not is_comment_line(x), + 'check': lambda x: cast_whitespace_check(x), + 'print': + lambda: print_error("Inappropriate spacing around cast")}, + {'regex': r'(\.c|\.h)(\.in)?$', 'match_name': None, 'prereq': lambda x: not is_comment_line(x), 'check': lambda x: trailing_operator(x), @@ -565,7 +608,7 @@ checks = [ {'regex': r'(\.c|\.h)(\.in)?$', 'match_name': None, 'prereq': lambda x: has_comment(x), - 'check': lambda x: check_comment_spelling(x)}, + 'check': lambda x: check_spelling(x, True)}, {'regex': r'(\.c|\.h)(\.in)?$', 'match_name': None, 'check': lambda x: empty_return_with_brace(x), @@ -573,6 +616,11 @@ checks = [ 'print': lambda: print_warning("Empty return followed by brace, consider omitting") }, + + {'regex': r'(\.at|\.sh)$', 'match_name': None, + 'check': lambda x: has_efgrep(x), + 'print': + lambda: print_error("grep -E/-F should be used instead of egrep/fgrep")}, ] @@ -585,6 +633,10 @@ def regex_error_factory(description): return lambda: print_error(description) +def regex_warn_factory(description): + return lambda: print_warning(description) + + std_functions = [ ('malloc', 'Use xmalloc() in place of malloc()'), ('calloc', 'Use xcalloc() in place of calloc()'), @@ -601,6 +653,7 @@ std_functions = [ ('assert', 'Use ovs_assert() in place of assert()'), ('error', 'Use ovs_error() in place of error()'), ] + checks += [ {'regex': r'(\.c|\.h)(\.in)?$', 'match_name': None, @@ -609,6 +662,21 @@ checks += [ 'print': regex_error_factory(description)} for (function_name, description) in std_functions] +easy_to_misuse_api = [ + ('ovsrcu_barrier', + 'lib/ovs-rcu.c', + 'Are you sure you need to use ovsrcu_barrier(), ' + 'in most cases ovsrcu_synchronize() will be fine?'), + ] + +checks += [ + {'regex': r'(\.c)(\.in)?$', + 'match_name': lambda x: x != location, + 'prereq': lambda x: not is_comment_line(x), + 'check': regex_function_factory(function_name), + 'print': regex_warn_factory(description)} + for (function_name, location, description) in easy_to_misuse_api] + def regex_operator_factory(operator): regex = re.compile(r'^[^#][^"\']*[^ "]%s[^ "\'][^"]*' % operator) @@ -641,12 +709,22 @@ def get_file_type_checks(filename): global checks checkList = [] for check in checks: + regex_check = True + match_check = True + if check['regex'] is None and check['match_name'] is None: checkList.append(check) + continue + if check['regex'] is not None and \ - re.compile(check['regex']).search(filename) is not None: - checkList.append(check) - elif check['match_name'] is not None and check['match_name'](filename): + re.compile(check['regex']).search(filename) is None: + regex_check = False + + if check['match_name'] is not None and \ + not check['match_name'](filename): + match_check = False + + if regex_check and match_check: checkList.append(check) return checkList @@ -728,15 +806,31 @@ def ovs_checkpatch_parse(text, filename, author=None, committer=None): re.I | re.M | re.S) is_gerrit_change_id = re.compile(r'(\s*(change-id: )(.*))$', re.I | re.M | re.S) + is_fixes = re.compile(r'(\s*(Fixes:)(.*))$', re.I | re.M | re.S) + is_fixes_exact = re.compile(r'^Fixes: [0-9a-f]{12} \(".*"\)$') + + tags_typos = { + r'^Acked by:': 'Acked-by:', + r'^Reported at:': 'Reported-at:', + r'^Reported by:': 'Reported-by:', + r'^Requested by:': 'Requested-by:', + r'^Reviewed by:': 'Reviewed-by:', + r'^Submitted at:': 'Submitted-at:', + r'^Suggested by:': 'Suggested-by:', + } reset_counters() - for line in text.splitlines(): + for line in text.split("\n"): if current_file != previous_file: previous_file = current_file lineno = lineno + 1 total_line = total_line + 1 + + if line == "\f": + # Form feed + continue if len(line) <= 0: continue @@ -811,10 +905,26 @@ def ovs_checkpatch_parse(text, filename, author=None, committer=None): elif is_co_author.match(line): m = is_co_author.match(line) co_authors.append(m.group(2)) - elif is_gerrit_change_id.match(line): + elif (is_gerrit_change_id.match(line) and + not skip_gerrit_change_id_check): print_error( "Remove Gerrit Change-Id's before submitting upstream.") print("%d: %s\n" % (lineno, line)) + elif is_fixes.match(line) and not is_fixes_exact.match(line): + print_error('"Fixes" tag is malformed.\n' + 'Use the following format:\n' + ' git log -1 ' + '--pretty=format:"Fixes: %h (\\\"%s\\\")" ' + '--abbrev=12 COMMIT_REF\n') + print("%d: %s\n" % (lineno, line)) + elif spellcheck: + check_spelling(line, False) + for typo, correct in tags_typos.items(): + m = re.match(typo, line, re.I) + if m: + print_error("%s tag is malformed." % (correct[:-1])) + print("%d: %s\n" % (lineno, line)) + elif parse == PARSE_STATE_CHANGE_BODY: newfile = hunks.match(line) if newfile: @@ -850,6 +960,8 @@ def ovs_checkpatch_parse(text, filename, author=None, committer=None): # for a common style. if current_file.startswith('include/sparse'): continue + if current_file.startswith('utilities/bugtool'): + continue run_checks(current_file, cmp_line, lineno) run_file_checks(text) @@ -875,8 +987,10 @@ Check options: -l|--skip-leading-whitespace Skips the leading whitespace test -q|--quiet Only print error and warning information -s|--skip-signoff-lines Tolerate missing Signed-off-by line --S|--spellcheck-comments Check C comments for possible spelling mistakes --t|--skip-trailing-whitespace Skips the trailing whitespace test""" +-S|--spellcheck Check C comments and commit-message for possible + spelling mistakes +-t|--skip-trailing-whitespace Skips the trailing whitespace test + --skip-gerrit-change-id Skips the gerrit change id test""" % sys.argv[0]) @@ -892,7 +1006,7 @@ def ovs_checkpatch_print_result(): def ovs_checkpatch_file(filename): try: - mail = email.message_from_file(open(filename, 'r')) + mail = email.message_from_file(open(filename, 'r', encoding='utf8')) except: print_error("Unable to parse file '%s'. Is it a patch?" % filename) return -1 @@ -933,7 +1047,8 @@ if __name__ == '__main__': "skip-leading-whitespace", "skip-signoff-lines", "skip-trailing-whitespace", - "spellcheck-comments", + "skip-gerrit-change-id", + "spellcheck", "quiet"]) except: print("Unknown option encountered. Please rerun with -h for help.") @@ -951,14 +1066,16 @@ if __name__ == '__main__': skip_signoff_check = True elif o in ("-t", "--skip-trailing-whitespace"): skip_trailing_whitespace_check = True + elif o in ("--skip-gerrit-change-id"): + skip_gerrit_change_id_check = True elif o in ("-f", "--check-file"): checking_file = True - elif o in ("-S", "--spellcheck-comments"): + elif o in ("-S", "--spellcheck"): if not open_spell_check_dict(): print("WARNING: The enchant library isn't available.") print(" Please install python enchant.") else: - spellcheck_comments = True + spellcheck = True elif o in ("-q", "--quiet"): quiet = True else: