From patchwork Tue Jul 31 20:37:38 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bala Sankaran X-Patchwork-Id: 951820 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 41g7bb3njvz9s7Q for ; Wed, 1 Aug 2018 06:39:55 +1000 (AEST) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id CAA2FFB7; Tue, 31 Jul 2018 20:37:41 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id A22B0F6A for ; Tue, 31 Jul 2018 20:37:40 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from mx1.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 0B4507A7 for ; Tue, 31 Jul 2018 20:37:39 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id E366E8197008 for ; Tue, 31 Jul 2018 20:37:38 +0000 (UTC) Received: from localhost.localdomain (unknown [10.18.25.187]) by smtp.corp.redhat.com (Postfix) with ESMTP id CE9661C5AF for ; Tue, 31 Jul 2018 20:37:38 +0000 (UTC) From: Bala Sankaran To: dev@openvswitch.org Date: Tue, 31 Jul 2018 16:37:38 -0400 Message-Id: <20180731203738.6282-1-bsankara@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.11.54.5 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.8]); Tue, 31 Jul 2018 20:37:38 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.8]); Tue, 31 Jul 2018 20:37:38 +0000 (UTC) for IP:'10.11.54.5' DOMAIN:'int-mx05.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'bsankara@redhat.com' RCPT:'' X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Subject: [ovs-dev] [PATCH] checkpatch: warn on possible bare return X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org void functions do not need to have a return statement, because such statements are redundant. Warn the user of such instances. An interim line check is added to allow gathering additional context for each line that is being processed. Signed-off-by: Bala Sankaran --- utilities/checkpatch.py | 72 +++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 70 insertions(+), 2 deletions(-) diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py index f929714..b2f27f5 100755 --- a/utilities/checkpatch.py +++ b/utilities/checkpatch.py @@ -74,8 +74,12 @@ try: except: no_spellcheck = True +RETURN_CHECK_INITIAL_STATE = 0 +RETURN_CHECK_STATE_WITH_RETURN = 1 +RETURN_CHECK_AWAITING_BRACE = 2 __errors = 0 __warnings = 0 +empty_return_check_state = 0 print_file_name = None checking_file = False total_line = 0 @@ -158,6 +162,7 @@ __regex_conditional_else_bracing2 = re.compile(r'^\s*}\selse\s*$') __regex_has_xxx_mark = re.compile(r'.*xxx.*', re.IGNORECASE) __regex_added_doc_rst = re.compile( r'\ndiff .*Documentation/.*rst\nnew file mode') +__regex_empty_return = re.compile(r'\s*return;') skip_leading_whitespace_check = False skip_trailing_whitespace_check = False @@ -454,6 +459,33 @@ def check_new_docs_index(text): return __check_new_docs(text, 'rst') +def empty_return_with_brace(line): + """Returns TRUE if a function contains a return; followed + by one or more line feeds and terminates with a '}' + at start of line""" + + def empty_return(line): + """Returns TRUE if a function has a 'return;'""" + return __regex_empty_return.match(line) is not None + + global empty_return_check_state + if empty_return_check_state == RETURN_CHECK_INITIAL_STATE \ + and empty_return(line): + empty_return_check_state = RETURN_CHECK_STATE_WITH_RETURN + elif empty_return_check_state == RETURN_CHECK_STATE_WITH_RETURN \ + and (re.match(r'^}$', line) or len(line) == 0): + if re.match('^}$', line): + empty_return_check_state = RETURN_CHECK_AWAITING_BRACE + else: + empty_return_check_state = RETURN_CHECK_INITIAL_STATE + + if empty_return_check_state == RETURN_CHECK_AWAITING_BRACE: + empty_return_check_state = RETURN_CHECK_INITIAL_STATE + return True + + return False + + file_checks = [ {'regex': __regex_added_doc_rst, 'check': check_new_docs_index}, @@ -505,6 +537,13 @@ checks = [ {'regex': '(\.c|\.h)(\.in)?$', 'match_name': None, 'prereq': lambda x: has_comment(x), 'check': lambda x: check_comment_spelling(x)}, + + {'regex': '(\.c|\.h)(\.in)?$', 'match_name': None, + 'check': lambda x: empty_return_with_brace(x), + 'interim_line': True, + 'print': + lambda: print_warning("Empty return followed by brace, consider omitting") + }, ] @@ -599,6 +638,29 @@ def run_checks(current_file, line, lineno): print("%s\n" % line) +def interim_line_check(current_file, line, lineno): + """Runs the various checks for the particular interim line. This will + take filename into account, and will check for the 'interim_line' + key before running the check.""" + global checking_file, total_line + print_line = False + for check in get_file_type_checks(current_file): + if 'prereq' in check and not check['prereq'](line): + continue + if 'interim_line' in check and check['interim_line']: + if check['check'](line): + if 'print' in check: + check['print']() + print_line = True + + if print_line: + if checking_file: + print("%s:%d:" % (current_file, lineno)) + else: + print("#%d FILE: %s:%d:" % (total_line, current_file, lineno)) + print("%s\n" % line) + + def run_file_checks(text): """Runs the various checks for the text.""" for check in file_checks: @@ -607,7 +669,8 @@ def run_file_checks(text): def ovs_checkpatch_parse(text, filename): - global print_file_name, total_line, checking_file + global print_file_name, total_line, checking_file, \ + empty_return_check_state PARSE_STATE_HEADING = 0 PARSE_STATE_DIFF_HEADER = 1 @@ -680,17 +743,22 @@ def ovs_checkpatch_parse(text, filename): continue reset_line_number = hunk_differences.match(line) if reset_line_number: + empty_return_check_state = RETURN_CHECK_INITIAL_STATE lineno = int(reset_line_number.group(3)) if lineno < 0: lineno = -1 * lineno lineno -= 1 + if is_subtracted_line(line): lineno -= 1 - if not is_added_line(line): continue cmp_line = added_line(line) + if not is_added_line(line): + interim_line_check(current_file, cmp_line, lineno) + continue + # Skip files which have /datapath in them, since they are # linux or windows coding standards if current_file.startswith('datapath'):