Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20181017031241.669-1-kys@linuxonhyperv.com> <20181017031406.773-1-kys@linuxonhyperv.com>
 <20181017031406.773-3-kys@linuxonhyperv.com> <20181017050705.GE22447@kroah.com>
 <20181017062255.oiu44y4zuuwilan3@mwanda>
In-Reply-To: <20181017062255.oiu44y4zuuwilan3@mwanda>
From:   Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
Date:   Sat, 20 Oct 2018 16:42:07 +0200
Message-ID: <CANiq72kQwhOFC5=vaNZW9qpmohot3ATRJVnn95A8eJ2w2xCNMQ@mail.gmail.com>
Subject: Re: [PATCH 3/5] Drivers: hv: kvp: Fix the recent regression caused by
 incorrect clean-up
To:     Dan <dan.carpenter@oracle.com>
Cc:     Greg KH <gregkh@linuxfoundation.org>, kys@microsoft.com,
        olaf@aepfle.de, sthemmin@microsoft.com, jasowang@redhat.com,
        linux-kernel <linux-kernel@vger.kernel.org>,
        Stable@vger.kernel.org, Michael.H.Kelley@microsoft.com,
        Robo Bot <apw@canonical.com>, devel@linuxdriverproject.org,
        vkuznets@redhat.com, haiyangz@microsoft.com
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

+On Wed, Oct 17, 2018 at 8:25 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> It's not common at all.  It should be wrapped in a macro and put into
> compiler.h.
>
> But I hope it does become adopted.  It's better than randomly grepping
> for non-standard comments.

Using an attribute is indeed better whenever possible. In C++17 it is
an standard attribute and there have been proposals to include some of
them for C as well since 2016 at least, e.g. the latest for
fallthrough at:

  http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2268.pdf

I have taken a quick look into supporting it (typing it here to save
it on the mailing list :-), and we have:

  * gcc >= 7.1 supports -Wimplicit-fallthrough with
__attribute__((fallthrough)), the comment syntax and the C++
[[fallthrough]] syntax.
  * gcc < 7.1 complains about empty declarations (it does not know
about attributes for null statements) and also
-Wdeclaration-after-statement.
  * clang 7 supports -Wimplicit-fallthrough (not enabled in
-Wall/extra/pedantic like gcc, though) but *only* in C++ mode and with
the C++ syntax [[fallthrough]]. In other words, in C mode, no syntax
works and no diagnostics are emitted. It complains about
Wmissing-declarations. [IMO they should allow the __attribute__ syntax
for fallthrough (and enable it on C mode) to be compatible with gcc.
Maybe they are simply waiting for the C2x attributes... :-)]
  * icc 19 does not know about -Wimplicit-fallthrough at all (but
seems to allow [[fallthrough]] on C++17 mode to comply with the
standard).

Therefore, the only improvement we could do right now is starting to
use the attribute for gcc > 7.1, and a comment for everybody else.
However, even if that was worth the trouble of changing the 2500+
instances of fall through markings that we have, comments are replaced
before the preprocessor stage, so we would need some (probably
non-portable) macro magic.

So, I would say, let's revisit this again in a few years. Possibly,
when we move the minimum version to gcc 7.1, clang and icc may both
support the fallthrough warning in C mode; so that we can always use
the __attribute__((fallthrough)) unconditionally under a __fallthrough
#define.

Cheers,
Miguel