From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1752108AbdDIJBl (ORCPT <rfc822;w@1wt.eu>);
        Sun, 9 Apr 2017 05:01:41 -0400
Received: from rhlx01.hs-esslingen.de ([129.143.116.10]:53888 "EHLO
        rhlx01.hs-esslingen.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1751814AbdDIJBi (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Sun, 9 Apr 2017 05:01:38 -0400
X-Greylist: delayed 366 seconds by postgrey-1.27 at vger.kernel.org; Sun, 09 Apr 2017 05:01:38 EDT
Date: Sun, 9 Apr 2017 10:55:31 +0200
From: Andreas Mohr <andi@lisas.de>
To: Joe Perches <joe@perches.com>
Cc: Alfonso Lima <alfonsolimaastor@gmail.com>, apw@canonical.com,
        linux-kernel@vger.kernel.org
Subject: Re: [bug report] checkpatch: if statement does not need to be
 enclosed in parentheses
Message-ID: <20170409085531.GA26418@rhlx01.hs-esslingen.de>
References: <20170408160717.GA7337@alima-W65-67SF>
 <1491672799.3250.41.camel@perches.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1491672799.3250.41.camel@perches.com>
X-Priority: none
User-Agent: Mutt/1.8.0 (2017-02-23)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Sat, Apr 08, 2017 at 10:33:19AM -0700, Joe Perches wrote:
> On Sat, 2017-04-08 at 17:07 +0100, Alfonso Lima wrote:
> > Hi,
> > 
> > In current linux-next, if you run the script on this piece of
> > code:
> > 
> > #define FOO(a)				\
> > 	if (a) {			\
> > 		something();		\
> > 		something_else();	\
> > 	}
> > 
> > You should get:
> > 
> > ERROR: Macros with complex values should be enclosed in parentheses
> > #1: FILE: foo.c:1:
> > +#define FOO(a)				\
> > +	if (a) {			\
> > +		something();		\
> > +		something_else();	\
> > +	}
> > 
> > We could silence checkpatch.pl using "do {} while ()" around the
> > if statement. However, the "if () {}" statement should be
> > enough. If someone could confirm this, I'll go and fix it.
> 
> Multiple if/else use is the reason do {} while (0) is suggested.
> 
> 	if (bar())
> 		FOO(a);
> 	else
> 		baz(b);
>  

So, if I read this right checkpatch.pl is generating a correct complaint
in this case after all?

However, then we seem to have the following issues remaining:
- "Macros with complex values should be enclosed in parentheses" != "do {} while (0) is suggested" --> this is an imprecision which may cause confusion
- *further* cases of this issue occurring may just as well suggest again
  "fixing" (i.e., "damaging") checkpatch.pl rather than
  simply obeying (following through on) its correct diagnosis
  --> checkpatch.pl behaviour is confusing enough that
  it may prompt incorrect core handling "repair actions",
  and this danger will remain permanently until
  checkpatch.pl reporting is improved to be
  less confusing/insufficient about its diagnosis

Or perhaps this imprecision here
again simply is due to
the very frequently observable case of
saying "thou shan't do this" without then also
giving some tried-and-true *reason(s)* for such advice.
("ERROR: Macros with complex values should be enclosed in parentheses")

HTH,

Andreas Mohr